[PATCH v2] v2v: rhv-upload-plugin: Fix name error
by Nir Soffer
"http" and "transfer" variables were missing in emulate_zero, so the
code would fail with NameError. This can happen only when communicating
with old imageio versions not supporting the "zero" feature.
Testing with qemu-img 2.12 show that we never send emulated zero request
because of the highestwrite mechanism, but it can break with older
qemu-img-rhev used on RHEL.
---
v2v/rhv-upload-plugin.py | 3 +++
1 file changed, 3 insertions(+)
diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py
index 2eec375f7..10887c031 100644
--- a/v2v/rhv-upload-plugin.py
+++ b/v2v/rhv-upload-plugin.py
@@ -338,6 +338,9 @@ def zero(h, count, offset, may_trim):
r.read()
def emulate_zero(h, count, offset):
+ http = h['http']
+ transfer = h['transfer']
+
# qemu-img convert starts by trying to zero/trim the whole device.
# Since we've just created a new disk it's safe to ignore these
# requests as long as they are smaller than the highest write seen.
--
2.17.1
6 years, 1 month
[PATCH] v2v: rhv-upload-plugin: Fix name error
by Nir Soffer
http was missing in emulate_zero, so the code would fail with NameError.
This can happen only when communicating with old imageio versions not
supporting the "zero" feature.
Testing with qemu-img 2.12 show that we never send emulated zero request
because of the highestwrite mechanism, but it can break with older
qemu-img-rhev used on RHEL.
---
v2v/rhv-upload-plugin.py | 2 ++
1 file changed, 2 insertions(+)
diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py
index 2eec375f7..0f6b31e67 100644
--- a/v2v/rhv-upload-plugin.py
+++ b/v2v/rhv-upload-plugin.py
@@ -338,6 +338,8 @@ def zero(h, count, offset, may_trim):
r.read()
def emulate_zero(h, count, offset):
+ http = h['http']
+
# qemu-img convert starts by trying to zero/trim the whole device.
# Since we've just created a new disk it's safe to ignore these
# requests as long as they are smaller than the highest write seen.
--
2.17.1
6 years, 1 month
[PATCH] v2v: rvh-upload-plugin: Always read the response
by Nir Soffer
Python manual warns[1]:
Note that you must have read the whole response before you can send
a new request to the server.
The reason for this warning is exposed only when the server is using
keep alive connections. When the response is not read, sending a new
request will fail with:
httplib.ResponseNotReady
Even if Content-Length was 0 or the request has no content. The failure
looks like this when using --verbose:
nbdkit: python[1]: debug: zero count=33554432 offset=0 may_trim=1 fua=0
nbdkit: python[1]: debug: zero count=33554432 offset=33554432 may_trim=1 fua=0
nbdkit: python[1]: error: /home/nsoffer/src/libguestfs/tmp/rhvupload.Au2B5I/rhv-upload-plugin.py: zero: error: Request-sent
nbdkit: python[1]: debug: sending error reply: Input/output error
qemu-img: error writing zeroes at offset 0: Input/output error
Change all requests to read the whole response.
Tested with imageio patch supporting keep alive connections:
https://gerrit.ovirt.org/#/c/92296/
[1] https://docs.python.org/3.8/library/http.client.html#http.client.HTTPConn...
---
v2v/rhv-upload-plugin.py | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py
index 7c5084efd..2eec375f7 100644
--- a/v2v/rhv-upload-plugin.py
+++ b/v2v/rhv-upload-plugin.py
@@ -197,11 +197,13 @@ def get_options(h):
http.endheaders()
r = http.getresponse()
+ data = r.read()
+
if r.status == 200:
# New imageio never needs authentication.
h['needs_auth'] = False
- j = json.loads(r.read())
+ j = json.loads(data)
h['can_zero'] = "zero" in j['features']
h['can_trim'] = "trim" in j['features']
h['can_flush'] = "flush" in j['features']
@@ -276,6 +278,7 @@ def pread(h, count, offset):
request_failed(h, r,
"could not read sector offset %d size %d" %
(offset, count))
+
return r.read()
def pwrite(h, buf, offset):
@@ -302,6 +305,8 @@ def pwrite(h, buf, offset):
"could not write sector offset %d size %d" %
(offset, count))
+ r.read()
+
def zero(h, count, offset, may_trim):
http = h['http']
transfer = h['transfer']
@@ -330,6 +335,8 @@ def zero(h, count, offset, may_trim):
"could not zero sector offset %d size %d" %
(offset, count))
+ r.read()
+
def emulate_zero(h, count, offset):
# qemu-img convert starts by trying to zero/trim the whole device.
# Since we've just created a new disk it's safe to ignore these
@@ -357,6 +364,8 @@ def emulate_zero(h, count, offset):
"could not write zeroes offset %d size %d" %
(offset, count))
+ r.read()
+
def trim(h, count, offset):
http = h['http']
transfer = h['transfer']
@@ -378,6 +387,8 @@ def trim(h, count, offset):
"could not trim sector offset %d size %d" %
(offset, count))
+ r.read()
+
def flush(h):
http = h['http']
transfer = h['transfer']
@@ -394,6 +405,8 @@ def flush(h):
if r.status != 200:
request_failed(h, r, "could not flush")
+ r.read()
+
def delete_disk_on_failure(h):
disk_service = h['disk_service']
disk_service.remove()
--
2.17.1
6 years, 1 month
Build and testing issues
by Nir Soffer
While setting up a development environment on a clean Fedora 28 host,
I got some errors.
I followed the instructions in
http://libguestfs.org/guestfs-building.1.html
dnf builddep libguestfs
./autogen.sh
autogen.sh failed:
./configure: line 57694: syntax error near unexpected token `external'
./configure: line 57694: `AM_GNU_GETTEXT(external)'
Checking the entire log show that autoreconf failed, but the script
continued:
...
m4/guestfs-libraries.m4:151: warning: macro 'AM_GNU_GETTEXT' not found in
library
autoreconf: configure.ac: tracing
autoreconf: running: true --copy
m4/guestfs-libraries.m4:151: warning: macro 'AM_GNU_GETTEXT' not found in
library
autoreconf: running: /usr/bin/autoconf
configure:57694: error: possibly undefined macro: AM_GNU_GETTEXT
If this token and others are legitimate, please use m4_pattern_allow.
See the Autoconf documentation.
autoreconf: /usr/bin/autoconf failed with exit status: 1
We can fix this by adding "set -e" in bootstrap. Any reason not to do this?
I found old thread here about this:
https://gb.redhat.com/archives/libguestfs/2018-February/msg00017.html
Richard suggested to install gettext-devel, and it solve this issue after
git clean -dxf and running ./autogen.sh again.
So it seems that gettest-devel should be in build requirements.
After fixing the first issue I found that jansson-devel is also missing.
Should'nt we add these packages to the build requirements?
Next issues - trying to test upload to ovirt, I found that these
packages should be installed:
nbdkit
nbdkit-plugin-python3
python3-ovirt-engine-sdk4
Should we document these requirements in the rhv-upload-plugin?
Nir
6 years, 1 month
p2v: Allow virt-v2v input and output drivers containing '-' (RHBZ#1590220).
by Richard W.M. Jones
v1 was here:
https://www.redhat.com/archives/libguestfs/2018-June/msg00047.html
v1 -> v2:
- Add a regular expression in the virt-v2v modules code too.
- - -
I was planning a much more ambitious second version of this
patch (half written too) which had:
virt-v2v --describe-modules
that listed a big chunk of XML which virt-p2v would parse. These
would include the names of the available modules and the long
descriptions (currently hard-coded into virt-p2v p2v/gui.c).
However it was a bunch of tricky code and it would be difficult to
maintain interop with both old and new versions of virt-v2v. Also
there's a problem with translating strings. So in the end I wasn't
happy with it.
Rich.
6 years, 1 month
[PATCH] v2v: <File ovf:size> changed to the actual size (if known).
by Richard W.M. Jones
Note that this attribute is optional.
Thanks: Arik Hadas
---
v2v/create_ovf.ml | 11 ++++++++---
v2v/test-v2v-o-rhv.ovf.expected | 2 +-
v2v/test-v2v-o-rhv.sh | 1 +
v2v/test-v2v-o-vdsm-options.ovf.expected | 2 +-
v2v/test-v2v-o-vdsm-options.sh | 1 +
5 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/v2v/create_ovf.ml b/v2v/create_ovf.ml
index e0f81e0a2..9e0c772fd 100644
--- a/v2v/create_ovf.ml
+++ b/v2v/create_ovf.ml
@@ -819,12 +819,17 @@ and add_disks targets guestcaps output_alloc sd_uuid image_uuids vol_uuids
(* Add disk to <References/> node. *)
let disk =
- e "File" [
+ let attrs = ref [
"ovf:href", fileref;
"ovf:id", vol_uuid;
- "ovf:size", Int64.to_string ov.ov_virtual_size; (* NB: in bytes *)
"ovf:description", generated_by;
- ] [] in
+ ] in
+ (match t.target_actual_size with
+ | None -> ()
+ | Some actual_size ->
+ List.push_back attrs ("ovf:size", Int64.to_string actual_size)
+ );
+ e "File" !attrs [] in
append_child disk references;
(* Add disk to DiskSection. *)
diff --git a/v2v/test-v2v-o-rhv.ovf.expected b/v2v/test-v2v-o-rhv.ovf.expected
index 1deec9c9d..7bcc456c5 100644
--- a/v2v/test-v2v-o-rhv.ovf.expected
+++ b/v2v/test-v2v-o-rhv.ovf.expected
@@ -2,7 +2,7 @@
<ovf:Envelope xmlns:rasd='http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationS...' xmlns:vssd='http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettin...' xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance' xmlns:ovf='http://schemas.dmtf.org/ovf/envelope/1/' xmlns:ovirt='http://www.ovirt.org/ovf' ovf:version='0.9'>
<!-- generated by virt-v2v -->
<References>
- <File ovf:href='#DISK_ID#/#VOL_ID#' ovf:id='#VOL_ID#' ovf:size='536870912' ovf:description='generated by virt-v2v'/>
+ <File ovf:href='#DISK_ID#/#VOL_ID#' ovf:id='#VOL_ID#' ovf:description='generated by virt-v2v' ovf:size='#SIZE#'/>
</References>
<Section xsi:type='ovf:NetworkSection_Type'>
<Info>List of networks</Info>
diff --git a/v2v/test-v2v-o-rhv.sh b/v2v/test-v2v-o-rhv.sh
index c9ec0dbce..eb22ed95b 100755
--- a/v2v/test-v2v-o-rhv.sh
+++ b/v2v/test-v2v-o-rhv.sh
@@ -74,6 +74,7 @@ sed -i \
-e "s/$DISK_ID/#DISK_ID#/g" \
-e "s/$VM_ID/#VM_ID#/g" \
-e "s/$VOL_ID/#VOL_ID#/g" \
+ -e "s/\(<File.*ovf:size='\)[^']*/\1#SIZE#/g" \
-e 's/\(\<generated by virt-v2v\) [^ <'"'\""']*/\1/' \
-e 's/<rasd:InstanceId>'"$RE_UUID"'</<rasd:InstanceId>#UUID#</' \
-e 's/<\(rasd:\)\?\(CreationDate\|LastModified\|last_modified_date\)>[^<]*</<\1\2>#DATE#</' \
diff --git a/v2v/test-v2v-o-vdsm-options.ovf.expected b/v2v/test-v2v-o-vdsm-options.ovf.expected
index c2a3e336c..c1282c19b 100644
--- a/v2v/test-v2v-o-vdsm-options.ovf.expected
+++ b/v2v/test-v2v-o-vdsm-options.ovf.expected
@@ -2,7 +2,7 @@
<ovf:Envelope xmlns:rasd='http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationS...' xmlns:vssd='http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettin...' xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance' xmlns:ovf='http://schemas.dmtf.org/ovf/envelope/1/' xmlns:ovirt='http://www.ovirt.org/ovf' ovf:version='0.9'>
<!-- generated by virt-v2v -->
<References>
- <File ovf:href='VOL' ovf:id='VOL' ovf:size='536870912' ovf:description='generated by virt-v2v'/>
+ <File ovf:href='VOL' ovf:id='VOL' ovf:description='generated by virt-v2v' ovf:size='#SIZE#'/>
</References>
<NetworkSection>
<Info>List of networks</Info>
diff --git a/v2v/test-v2v-o-vdsm-options.sh b/v2v/test-v2v-o-vdsm-options.sh
index 2c9de5eb7..c70446aa1 100755
--- a/v2v/test-v2v-o-vdsm-options.sh
+++ b/v2v/test-v2v-o-vdsm-options.sh
@@ -82,6 +82,7 @@ RE_UUID='\<[0-9a-fA-F]\{8\}-[0-9a-fA-F]\{4\}-[0-9a-fA-F]\{4\}-[0-9a-fA-F]\{4\}-[
# Filter variable strings
sed -i \
+ -e "s/\(<File.*ovf:size='\)[^']*/\1#SIZE#/g" \
-e 's/\(\<generated by virt-v2v\) [^ <'"'\""']*/\1/' \
-e 's/<rasd:InstanceId>'"$RE_UUID"'</<rasd:InstanceId>#UUID#</' \
-e 's/<\(rasd:\)\?\(CreationDate\|LastModified\|last_modified_date\)>[^<]*</<\1\2>#DATE#</' \
--
2.16.2
6 years, 1 month
[PATCH] v2v: Add <Disk ovf:capacity> attribute containing disk virtual size.
by Richard W.M. Jones
Required ever since this change was made to oVirt:
https://gerrit.ovirt.org/#/c/91902/
Thanks: Arik Hadas
---
v2v/create_ovf.ml | 1 +
v2v/test-v2v-o-rhv.ovf.expected | 2 +-
v2v/test-v2v-o-vdsm-options.ovf.expected | 2 +-
3 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/v2v/create_ovf.ml b/v2v/create_ovf.ml
index ac3c61b13..e0f81e0a2 100644
--- a/v2v/create_ovf.ml
+++ b/v2v/create_ovf.ml
@@ -835,6 +835,7 @@ and add_disks targets guestcaps output_alloc sd_uuid image_uuids vol_uuids
| OVirt -> image_uuid
| RHVExportStorageDomain -> vol_uuid);
"ovf:size", Int64.to_string size_gb;
+ "ovf:capacity", Int64.to_string ov.ov_virtual_size;
"ovf:fileRef", fileref;
"ovf:parentRef", "";
"ovf:vm_snapshot_id", uuidgen ();
diff --git a/v2v/test-v2v-o-rhv.ovf.expected b/v2v/test-v2v-o-rhv.ovf.expected
index 342eb99d3..1deec9c9d 100644
--- a/v2v/test-v2v-o-rhv.ovf.expected
+++ b/v2v/test-v2v-o-rhv.ovf.expected
@@ -10,7 +10,7 @@
</Section>
<Section xsi:type='ovf:DiskSection_Type'>
<Info>List of Virtual Disks</Info>
- <Disk ovf:diskId='#VOL_ID#' ovf:size='1' ovf:fileRef='#DISK_ID#/#VOL_ID#' ovf:parentRef='' ovf:vm_snapshot_id='#UUID#' ovf:volume-format='RAW' ovf:volume-type='Sparse' ovf:format='http://en.wikipedia.org/wiki/Byte' ovf:disk-interface='VirtIO' ovf:disk-type='System' ovf:boot='True' ovf:actual_size='1'/>
+ <Disk ovf:diskId='#VOL_ID#' ovf:size='1' ovf:capacity='536870912' ovf:fileRef='#DISK_ID#/#VOL_ID#' ovf:parentRef='' ovf:vm_snapshot_id='#UUID#' ovf:volume-format='RAW' ovf:volume-type='Sparse' ovf:format='http://en.wikipedia.org/wiki/Byte' ovf:disk-interface='VirtIO' ovf:disk-type='System' ovf:boot='True' ovf:actual_size='1'/>
</Section>
<Content ovf:id='out' xsi:type='ovf:VirtualSystem_Type'>
<Name>windows</Name>
diff --git a/v2v/test-v2v-o-vdsm-options.ovf.expected b/v2v/test-v2v-o-vdsm-options.ovf.expected
index af1ca01db..c2a3e336c 100644
--- a/v2v/test-v2v-o-vdsm-options.ovf.expected
+++ b/v2v/test-v2v-o-vdsm-options.ovf.expected
@@ -10,7 +10,7 @@
</NetworkSection>
<DiskSection>
<Info>List of Virtual Disks</Info>
- <Disk ovf:diskId='IMAGE' ovf:size='1' ovf:fileRef='VOL' ovf:parentRef='' ovf:vm_snapshot_id='#UUID#' ovf:volume-format='COW' ovf:volume-type='Sparse' ovf:format='http://en.wikipedia.org/wiki/Byte' ovf:disk-interface='VirtIO' ovf:disk-type='System' ovf:boot='True' ovf:actual_size='1'/>
+ <Disk ovf:diskId='IMAGE' ovf:size='1' ovf:capacity='536870912' ovf:fileRef='VOL' ovf:parentRef='' ovf:vm_snapshot_id='#UUID#' ovf:volume-format='COW' ovf:volume-type='Sparse' ovf:format='http://en.wikipedia.org/wiki/Byte' ovf:disk-interface='VirtIO' ovf:disk-type='System' ovf:boot='True' ovf:actual_size='1'/>
</DiskSection>
<VirtualSystem ovf:id='VM'>
<Name>windows</Name>
--
2.16.2
6 years, 1 month
v2v: docs: Removed vsyscall support in Debian kernels requiring workaround (RHBZ#1592061).
by Richard W.M. Jones
Debian kernels have disabled legacy vsyscall page support meaning they
cannot runs binaries that predate c.2013. To enable it again you must
add vsyscall=emulate to the kernel command line when booting. It's
unclear why Debian disabled this as according to the documentation:
Disabling this option saves about 7K of kernel size and
possibly 4K of additional runtime pagetable memory.
(presumably the "runtime pagetable memory" is per-process?) It seems
to me like a rather poor trade-off for not being able to run binaries
which are just 5 years old.
While I was writing this patch I imagined a much more complex
automatic workaround where we would try to detect if the host kernel
had this problem and add the extra vsyscall=emulate parameter
automatically. ‘./configure’ might carry a binary (for x86_64 only?)
built to use the vsyscall page and try to run it under the host
kernel. If it failed it would suggest using the workaround.
This solution seems a bit complex, and so this patch simply updates
the documentation which hopefully will make the problem discoverable.
I don't know how widespread this problem is. We've not seen it in
Fedora/RHEL kernels AFAIK.
Rich.
6 years, 1 month
[PATCH] v2v: -o libvirt: Don't write only <vendor> without <model> (RHBZ#1591789).
by Richard W.M. Jones
Avoids the libvirt error:
error: XML error: CPU vendor specified without CPU model
---
v2v/create_libvirt_xml.ml | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/v2v/create_libvirt_xml.ml b/v2v/create_libvirt_xml.ml
index 582419f00..fbe90eeaa 100644
--- a/v2v/create_libvirt_xml.ml
+++ b/v2v/create_libvirt_xml.ml
@@ -51,15 +51,17 @@ let create_libvirt_xml ?pool source target_buses guestcaps
source.s_cpu_topology <> None then (
let cpu = ref [] in
- (match source.s_cpu_vendor with
- | None -> ()
- | Some vendor ->
- List.push_back cpu (e "vendor" [] [PCData vendor])
- );
- (match source.s_cpu_model with
- | None -> ()
- | Some model ->
+ (match source.s_cpu_vendor, source.s_cpu_model with
+ | None, None
+ (* Avoid libvirt error: "CPU vendor specified without CPU model" *)
+ | Some _, None -> ()
+ | None, Some model ->
List.push_back cpu (e "model" ["fallback", "allow"] [PCData model])
+ | Some vendor, Some model ->
+ List.push_back_list cpu [
+ e "vendor" [] [PCData vendor];
+ e "model" ["fallback", "allow"] [PCData model]
+ ]
);
(match source.s_cpu_topology with
| None -> ()
--
2.16.2
6 years, 1 month