[cross-project PATCH v2] NBD 64-bit extensions
by Eric Blake
This is a cover letter for a set of multi-project patch series all
designed to implement 64-bit operations in NBD, and demonstrate
interoperability of the new extension between projects.
v1 of the project was attempted nearly a year ago:
https://lists.nongnu.org/archive/html/qemu-devel/2021-12/msg00453.html
Since then, I've addressed a lot of preliminary cleanups in libnbd to
make it easier to test things, and incorporated review comments issued
on v1, including:
- no orthogonality between simple/structured replies and 64-bit mode:
once extended headers are negotiated, all transmission traffic (both
from client to server and server to client) uses just one header
size
- add support for the client to pass a payload on commands to the
server, and demonstrate it by further implementing a way to pass a
flag with NBD_CMD_BLOCK_STATUS that says the client is passing a
payload to request status of just a subset of the negotiated
contexts, rather than all possible contexts that were earlier
negotiated during NBD_OPT_SET_META_CONTEXT
- tweaks to the header layouts: tweak block status to provide 64-bit
flags values (although "base:allocation" will remain usable with
just 32-bit flags); reply with offset rather than padding and with
fields rearranged for maximal sharing between client and server
layouts
- word-smithing of the NBD proposed protocol; split things into
several smaller patches, where we can choose how much to take
- more unit tests added in qemu and libnbd
- the series end with RFC patches on whether to support 64-bit hole
responses to NBD_CMD_READ, even though our current limitations say
servers don't have to accept more than a 32M read request and
therefore will never have that big of a hole to read)
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
1 year, 9 months
[libnbd PATCH v2 0/3] Improve nbdsh -u handling
by Eric Blake
v1 was here:
https://listman.redhat.com/archives/libguestfs/2022-October/030216.html
Since then, I've incorporated changes based on Rich's feedback:
swap order of patches 2 and 3
less change in patch 1 (including no unsafe eval(%s) for --uri)
in patch 2, include -c in list of snippets to store, and use dict of
lambdas to map back to the desired action
Eric Blake (3):
nbdsh: Refactor handling of -c
nbdsh: Allow -u interleaved with -c
nbdsh: Improve --help and initial banner contents.
python/nbdsh.py | 142 +++++++++++++++++++++++++++------------------
sh/test-context.sh | 26 ++++-----
sh/test-error.sh | 37 +++++++-----
3 files changed, 119 insertions(+), 86 deletions(-)
--
2.38.1
1 year, 10 months
Proposal for tools to inspect drivers and inject Windows virtio drivers
by Richard W.M. Jones
We've had requests from products that use libguestfs & virt-v2v to
provide additional tooling for:
(a) Inspecting a virtual machine disk image to find out what virtual
devices it needs (ie. what drivers are installed in the guest).
(b) Taking a Windows disk image and injecting virtio drivers from the
virtio-win suite.
Virt-v2v does both operations as a part of importing guests from
VMware to KVM, but it doesn't expose these as separate operations.
- - -
For (a), you might run the tool against a disk image and it would
display various facts (similar to virt-inspector
https://libguestfs.org/virt-inspector.1.html):
$ virt-drivers -a linux.img
<operatingsystems>
<operatingsystem>
<root>/dev/sda2</root>
<boot_drivers>
[list of drivers from the initramfs in a format TBD]
</boot_drivers>
<drivers>
[list of kernel modules]
</drivers>
<boot_loader>
[extra stuff about the bootloader configuration,
list of kernels, default kernel, grub1 or grub2,
config file, ...]
</boot_loader>
I propose a completely new tool added to guestfs-tools to do this,
which will basically pull the current kernel module and grub analysis
code from virt-v2v into a new library in libguestfs-common.
For Windows we actually don't do this in virt-v2v at the moment, so
that would need to be completely new code, likely parsing the
DriverDatabase from the Windows registry.
- - -
For (b), you could specify the location of the Window disk image and
the virtio-win path/ISO to have it attempt to install the drivers in
the disk image:
$ virt-customize -a windows.img \
--inject-virtio-win /usr/share/virtio-win/virtio-win-1.2.3.iso
This would largely involve taking the current virtio-win code from
virt-v2v, turning it into a library, and then adding a new module into
libguestfs-common/mlcustomize. (It would be a good time to refactor
this code.)
- - -
Good? Bad? Let me know what you think ...
I think one large danger is that injecting virtio-win drivers into
existing Windows images is a very invasive operation with a large
potential to go wrong. It would be better to work with the tools that
create these images so that they're able to inject virtio-win drivers
at initial creation. (Or "Inbox" the drivers with Microsoft, but
there may be issues there).
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
1 year, 11 months
[PATCH] inspect: check presence of BIOS boot partition to handle BIOS+GPT setup
by Andrey Drobyshev
If the guest uses BIOS firmware but GPT partitioned disk, and in
addition has "bios, esp" flag enabled on its "/boot" partition, then
inspection wrongly detects UEFI firmware and v2v ends up with the error:
"error: no bootloader detected".
Let's fix this by checking for the presence of BIOS boot partition (0xef02
type for gdisk, "bios_grub" flag for parted), which is used to store a
bootloader code in BIOS+GPT configurations. If such a partition is
present, then it's likely a BIOS+GPT setup.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev(a)virtuozzo.com>
---
convert/inspect_source.ml | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/convert/inspect_source.ml b/convert/inspect_source.ml
index 056d0bca..6650e136 100644
--- a/convert/inspect_source.ml
+++ b/convert/inspect_source.ml
@@ -219,6 +219,9 @@ and list_applications g root = function
(* See if this guest could use UEFI to boot. It should use GPT and
* it should have an EFI System Partition (ESP).
*
+ * If it has a BIOS boot partition (BIOS+GPT setup), then [BIOS] is
+ * returned.
+ *
* If it has ESP(s), then [UEFI devs] is returned where [devs] is the
* list of at least one ESP.
*
@@ -226,9 +229,13 @@ and list_applications g root = function
*)
and get_firmware_bootable_device g =
let rec uefi_ESP_guid = "C12A7328-F81F-11D2-BA4B-00A0C93EC93B"
+ and bios_boot_guid = "21686148-6449-6E6F-744E-656564454649"
and is_uefi_ESP dev part =
let partnum = g#part_to_partnum part in
g#part_get_gpt_type dev partnum = uefi_ESP_guid
+ and is_bios_boot dev part =
+ let partnum = g#part_to_partnum part in
+ g#part_get_gpt_type dev partnum = bios_boot_guid
and parttype_is_gpt dev =
try g#part_get_parttype dev = "gpt"
with G.Error msg as exn ->
@@ -239,14 +246,26 @@ and get_firmware_bootable_device g =
and is_uefi_bootable_part part =
let dev = g#part_to_dev part in
parttype_is_gpt dev && is_uefi_ESP dev part
+ and is_bios_gpt_part part =
+ let dev = g#part_to_dev part in
+ parttype_is_gpt dev && is_bios_boot dev part
in
let partitions = Array.to_list (g#list_partitions ()) in
- let partitions = List.filter is_uefi_bootable_part partitions in
+ let esp_partitions = List.filter is_uefi_bootable_part partitions in
- match partitions with
- | [] -> I_BIOS
- | partitions -> I_UEFI partitions
+ (* If there's a BIOS boot partition present (0xef02 type for gdisk,
+ * "bios_grub" flag for parted), then this is likely a BIOS+GPT setup.
+ * Note that if a source VM is using UEFI firmware and has a secondary
+ * non-bootable disk attached which contains such a partition, the
+ * firmware detection will detect I_BIOS wrongly. But this can only be
+ * done manually, and we assume that there's no point doing it on purpose.
+ *)
+ if List.exists is_bios_gpt_part partitions then I_BIOS
+ else
+ match esp_partitions with
+ | [] -> I_BIOS
+ | esp_partitions -> I_UEFI esp_partitions
(* If some inspection fields are "unknown", then that indicates a
* failure in inspection, and we shouldn't continue. For an example
--
2.31.1
1 year, 11 months
[v2v PATCH] windows_virtio: favor "fwcfg" over "qemufwcfg"
by Laszlo Ersek
Virtio-win may provide the "qemufwcfg" stub driver and/or the "fwcfg"
actual driver. If both are provided, we must not install both, as they
conflict with each other. Pick "fwcfg" in this case.
Because the drivers can originate from two sources (libosinfo vs.
virtio-win), *and* because "copy_from_virtio_win" is reused for the QEMU
guest agent (i.e., not just for the drivers), do not sink the above
filtering into "copy_drivers" (or even more deeply). Instead, let copying
complete, and then clean up "driverdir" -- remove "qemufwcfg" if "fwcfg"
is present. (We'd have to consult the full list of drivers anyway, to see
if "fwcfg" indicates we should exclude "qemufwcfg".)
A note on annotating the "install_drivers" parameter list (OCaml obscurity
level one million):
> -let rec install_drivers ((g, _) as reg) inspect =
> +let rec install_drivers ((g, _) as reg : Registry.t) inspect =
Turns out that in this module, OCaml doesn't really have an idea that all
the "g#method" calls are made for an object of type "Guestfs.guestfs".
Instead, the compiler infers an interface from the methods we call, and
then tries to shoehorn that "collected interface" into "Guestfs.guestfs"
when it reaches:
> reg_import reg (regedits @ common_regedits)
This used to work fine until now; however, once we call
> g#glob_expand (driverdir // "qemufwcfg.*")
in this patch, the "reg_import" call fails like this:
> File "windows_virtio.ml", line 152, characters 13-16:
> 152 | reg_import reg (regedits @ common_regedits)
> ^^^
> Error: This expression has type
> (< [snip]
> glob_expand : string -> 'weak1 array;
> [snip] >
> as 'a) *
> 'weak2
> but an expression was expected of type
> Registry.t = Guestfs.guestfs * Registry.node
> Type
> < [snip]
> glob_expand : string -> 'weak1 array;
> [snip] >
> as 'a
> is not compatible with type
> Guestfs.guestfs =
> < [snip]
> glob_expand : ?directoryslash:bool -> string -> string array;
> [snip] >
> Types for method glob_expand are incompatible
The problem is that in our "g#glob_expand" call, we silently omit the
optional parameter "directoryslash", so at that point the compiler
"infers" a parameter list
> glob_expand : string -> 'weak1 array;
for the "interface" we require. And then that blows up because the actual
object provides only
> glob_expand : ?directoryslash:bool -> string -> string array;
The solution is to enlighten the compiler in "install_drivers" about the
actual type of "g", so that it need not infer or "collect" an interface
when we call "g#glob_expand" with "directoryslash" elided.
Now, "install_drivers" is called (as a callback!) ultimately from
"Registry.with_hive_write", and so its "reg" parameter has type
"Registry.t":
> type t = Guestfs.guestfs * node
(This is in fact reported by the compiler too, in the above-quoted error
message. Except said error message is like ten pages long -- see those
[snip] markers? --, so you will only ever find the relevant bit in the
error report if you already know what to look for. Helpful, that!)
Therefore, annotating the "reg" parameter like this:
> -let rec install_drivers ((g, _) as reg) inspect =
> +let rec install_drivers ((g, _) as reg : Registry.t) inspect =
forces "g" to have type "Guestfs.guestfs".
This is of course super obscure. The hint was that both
"linux_bootloaders.ml" and "convert_linux.ml" already used "g#glob_expand"
without the optional parameter -- and the important difference was that
these files had been type-annotated previously.
In *retrospect* -- that is, rather uselessly... --, the language
documentation does highlight this
<https://v2.ocaml.org/manual/lablexamples.html#s:label-inference>:
> We will not try here to explain in detail how type inference works. One
> must just understand that there is not enough information in the above
> program to deduce the correct type of g or bump. That is, there is no
> way to know whether an argument is optional or not, or which is the
> correct order, by looking only at how a function is applied. The
> strategy used by the compiler is to assume that there are no optional
> arguments, and that applications are done in the right order.
>
> [...]
>
> In practice, such problems appear mostly when using objects whose
> methods have optional arguments, so that writing the type of object
> arguments is often a good idea.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2151752
Signed-off-by: Laszlo Ersek <lersek(a)redhat.com>
---
convert/windows_virtio.ml | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/convert/windows_virtio.ml b/convert/windows_virtio.ml
index 3156694d114b..d9fda13f999b 100644
--- a/convert/windows_virtio.ml
+++ b/convert/windows_virtio.ml
@@ -44,7 +44,7 @@ let viostor_modern_pciid = "VEN_1AF4&DEV_1042&SUBSYS_11001AF4&REV_01"
let vioscsi_legacy_pciid = "VEN_1AF4&DEV_1004&SUBSYS_00081AF4&REV_00"
let vioscsi_modern_pciid = "VEN_1AF4&DEV_1048&SUBSYS_11001AF4&REV_01"
-let rec install_drivers ((g, _) as reg) inspect =
+let rec install_drivers ((g, _) as reg : Registry.t) inspect =
(* Copy the virtio drivers to the guest. *)
let driverdir = sprintf "%s/Drivers/VirtIO" inspect.i_windows_systemroot in
g#mkdir_p driverdir;
@@ -103,6 +103,23 @@ let rec install_drivers ((g, _) as reg) inspect =
else
Virtio_net in
+ (* The "fwcfg" driver binds the fw_cfg device for real, and provides three
+ * files -- ".cat", ".inf", ".sys". (Possibly ".pdb" too.)
+ *
+ * The "qemufwcfg" driver is only a stub driver; it placates Device Manager
+ * (hides the "unknown device" question mark) but does not actually drive
+ * the fw_cfg device. It provides two files only -- ".cat", ".inf".
+ *
+ * These drivers conflict with each other (RHBZ#2151752). If we've copied
+ * both (either from libosinfo of virtio-win), let "fwcfg" take priority:
+ * remove "qemufwcfg".
+ *)
+ if g#exists (driverdir // "fwcfg.inf") &&
+ g#exists (driverdir // "qemufwcfg.inf") then (
+ debug "windows: skipping qemufwcfg stub driver in favor of fwcfg driver";
+ Array.iter g#rm (g#glob_expand (driverdir // "qemufwcfg.*"))
+ );
+
(* Did we install the miscellaneous drivers? *)
let virtio_rng_supported = g#exists (driverdir // "viorng.inf") in
let virtio_ballon_supported = g#exists (driverdir // "balloon.inf") in
1 year, 11 months
[V2V PATCH v5] parse_libvirt_xml: look for manual firmware in "/domain/os/loader/@type"
by Andrey Drobyshev
According to [1], there're different ways to specify which firmware is
to be used by a libvirt-driven VM. Namely, there's an automatic
firmware selection, e.g.:
...
<os firmware='(bios|efi)'>
...
and a manual one, e.g.:
...
<os>
<loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader>
...
</os>
...
with the latter being a way to specify UEFI firmware. So let's add this
search path as well when parsing source VM's libvirt xml.
[1] https://libvirt.org/formatdomain.html#bios-bootloader
Co-authored-by: Laszlo Ersek <lersek(a)redhat.com>
Signed-off-by: Andrey Drobyshev <andrey.drobyshev(a)virtuozzo.com>
Originally-by: Denis Plotnikov <dplotnikov(a)virtuozzo.com>
---
input/parse_libvirt_xml.ml | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/input/parse_libvirt_xml.ml b/input/parse_libvirt_xml.ml
index 1e98ce1a..65693c98 100644
--- a/input/parse_libvirt_xml.ml
+++ b/input/parse_libvirt_xml.ml
@@ -446,12 +446,23 @@ let parse_libvirt_xml ?conn xml =
done;
List.rev !nics in
- (* Firmware. *)
+ (* Firmware.
+ * If "/domain/os" node doesn't contain "firmware" attribute (automatic
+ * firmware), we look for the presence of "pflash" in
+ * "/domain/os/loader/@type" attribute (manual firmware), with the latter
+ * indicating the UEFI firmware.
+ * See https://libvirt.org/formatdomain.html#bios-bootloader
+ *)
let firmware =
match xpath_string "/domain/os/@firmware" with
| Some "bios" -> BIOS
| Some "efi" -> UEFI
- | None | Some _ -> UnknownFirmware in
+ | Some _ -> UnknownFirmware
+ | None -> (
+ match xpath_string "/domain/os/loader/@type" with
+ | Some "pflash" -> UEFI
+ | _ -> UnknownFirmware
+ ) in
(* Check for hostdev devices. (RHBZ#1472719) *)
let () =
--
2.31.1
2 years
[V2V PATCH v4] parse_libvirt_xml: look for manual firmware in "/domain/os/loader/@type"
by Andrey Drobyshev
According to [1], there're different ways to specify which firmware is
to be used by a libvirt-driven VM. Namely, there's an automatic
firmware selection, e.g.:
...
<os firmware='(bios|efi)'>
...
and a manual one, e.g.:
...
<os>
<loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader>
...
</os>
...
with the latter being a way to specify UEFI firmware. So let's add this
search path as well when parsing source VM's libvirt xml.
[1] https://libvirt.org/formatdomain.html#bios-bootloader
Co-authored-by: Laszlo Ersek <lersek(a)redhat.com>
Signed-off-by: Andrey Drobyshev <andrey.drobyshev(a)virtuozzo.com>
Originally-by: Denis Plotnikov <dplotnikov(a)virtuozzo.com>
---
input/parse_libvirt_xml.ml | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/input/parse_libvirt_xml.ml b/input/parse_libvirt_xml.ml
index 56ce1c22..ab72c0ce 100644
--- a/input/parse_libvirt_xml.ml
+++ b/input/parse_libvirt_xml.ml
@@ -446,12 +446,23 @@ let parse_libvirt_xml ?conn xml =
done;
List.rev !nics in
- (* Firmware. *)
+ (* Firmware.
+ * If "/domain/os" node doesn't contain "firmware" attribute (automatic
+ * firmware), we look for the presence of "pflash" in
+ * "/domain/os/loader/@type" attribute (manual firmware), with the latter
+ * indicating the UEFI firmware.
+ * See https://libvirt.org/formatdomain.html#bios-bootloader
+ *)
let firmware =
match xpath_string "/domain/os/@firmware" with
| Some "bios" -> BIOS
| Some "efi" -> UEFI
- | None | Some _ -> UnknownFirmware in
+ | Some _ -> UnknownFirmware
+ | None -> (
+ match xpath_string "/domain/os/loader/@type" with
+ | Some "pflash" -> UEFI
+ | _ -> UnknownFirmware
+ ) in
(* Fallback to BIOS if we haven't found explicitly specified firmware.
* This is VZ-specific since we're either using "/domain/os/loader" node
--
2.31.1
2 years
[V2V PATCH v3] parse_libvirt_xml: look for manual firmware in "/domain/os/loader/@type"
by Andrey Drobyshev
According to [1], there're different ways to specify which firmware is
to be used by a libvirt-driven VM. Namely, there's an automatic
firmware selection, e.g.:
...
<os firmware='(bios|efi)'>
...
and a manual one, e.g.:
...
<os>
<loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader>
...
</os>
...
with the latter being a way to specify UEFI firmware. So let's add this
search path as well when parsing source VM's libvirt xml.
[1] https://libvirt.org/formatdomain.html#bios-bootloader
Signed-off-by: Andrey Drobyshev <andrey.drobyshev(a)virtuozzo.com>
Originally-by: Denis Plotnikov <dplotnikov(a)virtuozzo.com>
---
input/parse_libvirt_xml.ml | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/input/parse_libvirt_xml.ml b/input/parse_libvirt_xml.ml
index 1e98ce1a..449e3466 100644
--- a/input/parse_libvirt_xml.ml
+++ b/input/parse_libvirt_xml.ml
@@ -446,12 +446,28 @@ let parse_libvirt_xml ?conn xml =
done;
List.rev !nics in
- (* Firmware. *)
+ (* Firmware.
+ * If "/domain/os" node doesn't contain "firmware" attribute (automatic
+ * firmware), we look for the presence of "pflash" in
+ * "/domain/os/loader/@type" attribute (manual firmware), with the latter
+ * indicating the UEFI firmware.
+ * See https://libvirt.org/formatdomain.html#bios-bootloader
+ *)
let firmware =
match xpath_string "/domain/os/@firmware" with
| Some "bios" -> BIOS
| Some "efi" -> UEFI
- | None | Some _ -> UnknownFirmware in
+ | None | Some _ -> (
+ let loader = xpath_string "/domain/os/loader/@type" in
+ match loader with
+ | None -> UnknownFirmware
+ | _ -> (
+ let loader = Option.default "" loader in
+ match loader with
+ | "pflash" -> UEFI
+ | _ -> UnknownFirmware
+ )
+ ) in
(* Check for hostdev devices. (RHBZ#1472719) *)
let () =
--
2.31.1
2 years
[PATCH v2] parse_libvirt_xml: look for manual firmware in "/domain/os/loader"
by Andrey Drobyshev
According to [1], there're different ways to specify which firmware is
to be used by a libvirt-driven VM. Namely, there's an automatic
firmware selection, e.g.:
...
<os firmware='(bios|efi)'>
...
and a manual one, e.g.:
...
<os>
<loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader>
...
</os>
...
with the latter being a way to specify UEFI firmware. So let's add this
search path as well when parsing source VM's libvirt xml.
[1] https://libvirt.org/formatdomain.html#bios-bootloader
Signed-off-by: Andrey Drobyshev <andrey.drobyshev(a)virtuozzo.com>
Originally-by: Denis Plotnikov <dplotnikov(a)virtuozzo.com>
---
input/parse_libvirt_xml.ml | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/input/parse_libvirt_xml.ml b/input/parse_libvirt_xml.ml
index 1e98ce1a..0fecde33 100644
--- a/input/parse_libvirt_xml.ml
+++ b/input/parse_libvirt_xml.ml
@@ -27,6 +27,9 @@ open Xpath_helpers
open Types
open Utils
+(* Detect that "/domain/os/loader" node contains path to UEFI firmware. *)
+let loader_contains_ovmf_re = PCRE.compile "/OVMF_CODE"
+
type disk = {
d_format : string option; (* Disk format from XML if known. *)
d_type : disk_type; (* Disk type and extra information. *)
@@ -446,12 +449,26 @@ let parse_libvirt_xml ?conn xml =
done;
List.rev !nics in
- (* Firmware. *)
+ (* Firmware.
+ * If "/domain/os" node doesn't contain "firmware" attribute (automatic
+ * firmware), we look for the presence of "OVMF_CODE" in "/domain/os/loader"
+ * node (manual firmware).
+ * See https://libvirt.org/formatdomain.html#bios-bootloader
+ *)
let firmware =
match xpath_string "/domain/os/@firmware" with
| Some "bios" -> BIOS
| Some "efi" -> UEFI
- | None | Some _ -> UnknownFirmware in
+ | None | Some _ -> (
+ let loader = xpath_string "/domain/os/loader" in
+ match loader with
+ | None -> UnknownFirmware
+ | _ -> (
+ let loader = Option.default "" loader in
+ if PCRE.matches loader_contains_ovmf_re loader then UEFI
+ else UnknownFirmware
+ )
+ ) in
(* Check for hostdev devices. (RHBZ#1472719) *)
let () =
--
2.31.1
2 years