Re: [PATCH 1/4] conf: add loader type 'none'
On Tue, May 23, 2023 at 06:32:07PM +0300, David Abdurachmanov wrote: > On Tue, May 23, 2023 at 6:12 PM Andrea Bolognani wrote: > > > > On Tue, May 23, 2023 at 04:05:04PM +0300, David Abdurachmanov wrote: > > > On Tue, May 23, 2023 at 3:18 PM Andrea Bolognani > > > wrote: > > > > On Tue, May 23, 2023 at 11:59:41AM +0100, Richard W.M. Jones wrote: > > > > > I just came across this thread while trying to update the libvirt > > > > > instructions on that page. Specifically I need to add these to the > > > > > qemu command line: > > > > > > > > > > -bios /path/to/u-boot-spl.bin > > > > > -device loader,file=/path/to/u-boot.itb,addr=0x8020 > > > > > > > > Anyway, we found that the -device loader incantation above is > > > > effectively equivalent to -kernel, so you can just use that instead. > > > > Both -bios and -kernel are exposed by libvirt through XML elements of > > > > the same names. > > > > > > > > Further, since QEMU loads OpenSBI as -bios by default these days, you > > > > don't even need that part and can just use -kernel to point to the > > > > u-boot.bin file. In this case, you should use the file that would be > > > > installed as > > > > > > We run QEMU in a similar way as any other board. > > > > You mean physical boards? > > Yes > > > > > > So it's U-Boot SPL, which then loads U-Boot ITB (which typically > > > contains U-Boot proper, OpenSBI FW_DYNAMIC generic binary, and DTB). > > > U-Boot SPL transfers control to OpenSBI and tells it how to load the > > > next stage (i.e U-Boot proper). > > > > That sounds a bit roundabout when you consider that QEMU > > automatically loads OpenSBI, and that in turn knows to jump to a > > S-Mode payload such as the Linux kernel or an S-Mode build of U-Boot. > > > > Put it another way: do you have any objections to loading > > qemu-riscv64_smode/u-boot.bin via -kernel as the default boot > > strategy for riscv64 VMs? That's what every OS other than Fedora > > already recommends doing. > > We typically use a non-release version of OpenSBI, because it has the > latest hardware support and maybe some fixes. > > For libvirt users (out-of-the box experience) that's the correct approach. > > > > > Loading the SPL and ITB separately would still be possible, of > > course. I'm simply talking about the default experience. > > Yeah, this default is sane. > > > > > > > /usr/share/uboot/qemu-riscv64_smode/u-boot.bin > > > > > > > > by the uboot-images-riscv64 Fedora package. > > > > > > This is noarch package, and thus available on all arches. This package > > > is only available in Fedora/RISCV Koji for now. > > > > You're right! I got confused because "riscv64" is in the name O:-) > > > > Is there ongoing work to move this package to Fedora proper? In terms > > of user experience, having to download disk images from a third-party > > is mostly fine, but installing third-party RPMs on the host... Not so > > much. Having this in Fedora proper would make it a fully > > self-contained host for RISC-V TCG VMs. > > It could go as-is, but most likely will need to change. So far we > relied on a single OpenSBI FW_DYNAMIC generic image, but that changes > starting with JH7110 which has a different (i.e. non default) load > address. Thus we might need to build OpenSBI per board (or similar), > and pick the right one while building U-Boot. > > Unknown (at least for me) how things will work on TH1520 based boards too. > > One thing I am sure about is that things in the SPEC file most likely > will change once more boards from multiple vendors are supported. > > We can push the current thing as-is if you want. The "hump" here is getting it through the Fedora approval process. Once in we can add additional features very easily. If you want me to do this let me know, I can start right away (still fiddling around with benchmarking that qemu instance so I'm not fully occupied right now). Rich. > > > > > > > For the benefit of the search engine gods, this works for now: > > > > > > > > > > # virt-install --import --memory 8192 -n fedora-37-riscv \ > > > > > --arch riscv64 --vcpus 8 \ > > > > > --disk fedora-37-riscv.qcow2,format=qcow2 \ > > > > > --osinfo fedora37 \ > > > > > --qemu-commandline=' -bios /path/to/u
Re: [PATCH 1/4] conf: add loader type 'none'
On Tue, May 23, 2023 at 08:12:17AM -0700, Andrea Bolognani wrote: > On Tue, May 23, 2023 at 04:05:04PM +0300, David Abdurachmanov wrote: > > On Tue, May 23, 2023 at 3:18 PM Andrea Bolognani > > wrote: > > > On Tue, May 23, 2023 at 11:59:41AM +0100, Richard W.M. Jones wrote: > > > > I just came across this thread while trying to update the libvirt > > > > instructions on that page. Specifically I need to add these to the > > > > qemu command line: > > > > > > > > -bios /path/to/u-boot-spl.bin > > > > -device loader,file=/path/to/u-boot.itb,addr=0x8020 > > > > > > Anyway, we found that the -device loader incantation above is > > > effectively equivalent to -kernel, so you can just use that instead. > > > Both -bios and -kernel are exposed by libvirt through XML elements of > > > the same names. > > > > > > Further, since QEMU loads OpenSBI as -bios by default these days, you > > > don't even need that part and can just use -kernel to point to the > > > u-boot.bin file. In this case, you should use the file that would be > > > installed as > > > > We run QEMU in a similar way as any other board. > > You mean physical boards? > > > So it's U-Boot SPL, which then loads U-Boot ITB (which typically > > contains U-Boot proper, OpenSBI FW_DYNAMIC generic binary, and DTB). > > U-Boot SPL transfers control to OpenSBI and tells it how to load the > > next stage (i.e U-Boot proper). > > That sounds a bit roundabout when you consider that QEMU > automatically loads OpenSBI, and that in turn knows to jump to a > S-Mode payload such as the Linux kernel or an S-Mode build of U-Boot. > > Put it another way: do you have any objections to loading > qemu-riscv64_smode/u-boot.bin via -kernel as the default boot > strategy for riscv64 VMs? That's what every OS other than Fedora > already recommends doing. > > Loading the SPL and ITB separately would still be possible, of > course. I'm simply talking about the default experience. > > > > /usr/share/uboot/qemu-riscv64_smode/u-boot.bin > > > > > > by the uboot-images-riscv64 Fedora package. > > > > This is noarch package, and thus available on all arches. This package > > is only available in Fedora/RISCV Koji for now. > > You're right! I got confused because "riscv64" is in the name O:-) > > Is there ongoing work to move this package to Fedora proper? In terms > of user experience, having to download disk images from a third-party > is mostly fine, but installing third-party RPMs on the host... Not so > much. Having this in Fedora proper would make it a fully > self-contained host for RISC-V TCG VMs. In the interesting of being useful for once I could do this. What do you think David? > > > > For the benefit of the search engine gods, this works for now: > > > > > > > > # virt-install --import --memory 8192 -n fedora-37-riscv \ > > > > --arch riscv64 --vcpus 8 \ > > > > --disk fedora-37-riscv.qcow2,format=qcow2 \ > > > > --osinfo fedora37 \ > > > > --qemu-commandline=' -bios /path/to/u-boot-spl.bin -device > > > > loader,file=/path/to/u-boot.itb,addr=0x8020 ' > > > > This doesn't disable Sv48 and Sv57. I don't know the overall status, > > but at least Golang 1.20 has a fix to support anything above Sv39. > > > > Not sure about other runtimes that do pointer tagging. > > > > See: https://bugs.launchpad.net/ubuntu/+source/linux-riscv/+bug/1991790 > > IIUC that's something that needs to be handled in the kernel? Or do > we need to do something at the QEMU/libvirt/virt-install level to > make things work? David mentioned these CPU flags [qemu option]: -cpu rv64,sv57=off,sv48=off However it seems this is only a temporary workaround until various language runtimes are fixed. > > Simply put, older disk images on newer QEMU versions might not work > > properly. > > I'm not too concerned about this. In the long run, it will simply not > matter. > > > Note that we might want to switch to EDK2 in the future for QEMU, and > > that probably will use two 32MiB pflash devices in virt machine. I > > have seen, but haven't tested QEMU + EDK2 patches. > > Yup, there's some discussion about this very topic happening right > now on the QEMU mailing list. I'm actively involved in it and it > looks like the necessary changes might be merged soon. > > I'm not sure we can count on EFI-enabled RISC-V disk images popping > up very quickly though, so I would like to improve the situation for > the disk images that are out there right now and need U-Boot to run. 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
Re: [PATCH 1/4] conf: add loader type 'none'
On Tue, May 23, 2023 at 04:05:04PM +0300, David Abdurachmanov wrote: > We run QEMU in a similar way as any other board. > > So it's U-Boot SPL, which then loads U-Boot ITB (which typically > contains U-Boot proper, OpenSBI FW_DYNAMIC generic binary, and DTB). > U-Boot SPL transfers control to OpenSBI and tells it how to load the > next stage (i.e U-Boot proper). Is there any documentation on how RISC-V boots now? And about the eventual goal that we're aiming for? > > > For the benefit of the search engine gods, this works for now: > > > > > > # virt-install --import --memory 8192 -n fedora-37-riscv \ > > > --arch riscv64 --vcpus 8 \ > > > --disk fedora-37-riscv.qcow2,format=qcow2 \ > > > --osinfo fedora37 \ > > > --qemu-commandline=' -bios /path/to/u-boot-spl.bin -device > > > loader,file=/path/to/u-boot.itb,addr=0x8020 ' > > This doesn't disable Sv48 and Sv57. I don't know the overall status, > but at least Golang 1.20 has a fix to support anything above Sv39. > > Not sure about other runtimes that do pointer tagging. > > See: https://bugs.launchpad.net/ubuntu/+source/linux-riscv/+bug/1991790 Interesting and informative bug. I guess I'm not using any golang binaries, as the guest totally seems to work fine. I'm a bit confused how this kernel detail leaks into userspace though. I thought userspace can use all 64 bits in pointers. > Simply put, older disk images on newer QEMU versions might not work properly. > > Note that we might want to switch to EDK2 in the future for QEMU, and > that probably will use two 32MiB pflash devices in virt machine. I > have seen, but haven't tested QEMU + EDK2 patches. Rich. > Cheers, > david > > > > > Based on the information above, using > > > > --boot kernel=/path/to/u-boot.bin > > > > should work... > > > > > 'Course you have to disable SELinux ... > > > > ... without requiring this :) > > > > -- > > Andrea Bolognani / Red Hat / Virtualization > > -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Re: [PATCH 1/4] conf: add loader type 'none'
On Wed, Mar 22, 2023 at 11:37:10AM -0500, Andrea Bolognani wrote: > On Wed, Mar 22, 2023 at 10:36:20AM -0300, Daniel Henrique Barboza wrote: > > I'm not sure if the OS overwrites the firmware when running bare metal. > > Usually > > they provide different OS images for QEMU/libvirt and bare metal systems, > > probably > > to account for these differences. Baking the firmware in the kernel like > > Fedora > > Rawhide is doing was important a few years ago when RISC-V QEMU wasn't > > loading > > the firmware by default, but now it's getting in the way. > > > > All this said, having a look at the recently updated Fedora for RISC-V > > wiki, the > > instructions for booting with libvirt and QEMU are different. libvirt [1] > > is using > > the '-bios none' attribute with 'virt-install' + a Fedora Rawhide image, but > > QEMU instructions [2] doesn't use '-bios none' and it's using Fedora 37. > > The libvirt instructions in that page seem to have been updated in > the Fedora 30 timeframe, whereas the QEMU instructions appear to be > current as of Fedora 37. > > > At first I don't see any particular reason of why this Fedora 37 image > > would work > > on QEMU and not on libvirt. So what t I'll do now is do some testings with > > libvirt > > and Fedora 37. If it works then this series becomes way less important. > > The libvirt instructions tell you to use > > --boot > kernel=Fedora-Developer-Rawhide-*-fw_payload-uboot-qemu-virt-smode.elf > --qemu-commandline='-bios none' > > (aka -bios none -kernel foo) while the QEMU ones suggest > > -bios u/usr/share/uboot/qemu-riscv64_spl/u-boot-spl.bin > -device > loader,file=u/usr/share/uboot/qemu-riscv64_spl/u-boot.itb,addr=0x8020 > > Those are completely different approaches to booting the guest OS. > The latter is much saner IMO, because it keeps the firmware under > control of the host (as is the case for SeaBIOS/edk2) instead of > mixing the kernel and the firmware and requiring guest-specific files > to be extracted from the disk image before being able to boot. > > libvirt already provides the ability to pass arbitrary paths to -bios > via the element, so making the first part work > should just be a matter of pointing it to the correct path. We don't > seem to have the -device loader part wired up though, so that would > need to be added. Probably as an additional attribute, in the vein of > nvram.template? The address would have to be specified as well. > > Note that the firmware descriptor format already supports u-boot as > the firmware type. So in the long run ideally you'd only need to > specify > > > > and, assuming the uboot-images-riscv64 package is installed on the > host, everything should just work. I just came across this thread while trying to update the libvirt instructions on that page. Specifically I need to add these to the qemu command line: -bios /path/to/u-boot-spl.bin -device loader,file=/path/to/u-boot.itb,addr=0x8020 Was any change made to libvirt / virt-install to make this possible? - - - For the benefit of the search engine gods, this works for now: # virt-install --import --memory 8192 -n fedora-37-riscv \ --arch riscv64 --vcpus 8 \ --disk fedora-37-riscv.qcow2,format=qcow2 \ --osinfo fedora37 \ --qemu-commandline=' -bios /path/to/u-boot-spl.bin -device loader,file=/path/to/u-boot.itb,addr=0x8020 ' 'Course you have to disable SELinux ... Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Re: [libvirt PATCH v3 09/18] qemu: add functions to start and stop nbdkit
On Fri, Dec 09, 2022 at 06:23:59PM +, Richard W.M. Jones wrote:
> $ guestfish --format=raw -a ssh://foo/var/tmp/fedora-36.img -i -vx
>
> ...
>
>
>
>
>
>
>
>
>
Actually I remembered in the "read-only"[1] case we do create a qcow2
overlay:
$ guestfish --ro --format=raw -a ssh://foo/var/tmp/fedora-36.img -i
...
libguestfs: trace: disk_create "/tmp/libguestfszdXqqC/overlay1.qcow2" "qcow2"
-1 "backingfile:ssh://foo/var/tmp/fedora-36.img" "backingformat:raw"
libguestfs: command: run: qemu-img
libguestfs: command: run: \ create
libguestfs: command: run: \ -f qcow2
libguestfs: command: run: \ -o
backing_file=ssh://foo/var/tmp/fedora-36.img,backing_fmt=raw
libguestfs: command: run: \ /tmp/libguestfszdXqqC/overlay1.qcow2
Formatting '/tmp/libguestfszdXqqC/overlay1.qcow2', fmt=qcow2 cluster_size=65536
extended_l2=off compression_type=zlib size=6442450944
backing_file=ssh://foo/var/tmp/fedora-36.img backing_fmt=raw lazy_refcounts=off
refcount_bits=16
...
which might be what you were thinking about. Actually this doesn't
work either:
Original error from libvirt: internal error: process exited while connecting to
monitor: 2022-12-09T20:17:41.074400Z qemu-kvm: -blockdev
{"driver":"ssh","path":"var/tmp/fedora-36.img","server":{"host":"foo","port":"22"},"node-name":"libvirt-4-storage","cache":{"direct":false,"no-flush":true},"auto-read-only":true,"discard":"unmap"}:
failed to authenticate using publickey authentication and the identities held
by your ssh-agent [code=1 int1=-1]
Rich.
[1] Not really "read-only" - we must create a r/w overlay because
otherwise replaying journals in filesystems does not work. The
overlay is discarded afterwards.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org
Re: [libvirt PATCH v3 09/18] qemu: add functions to start and stop nbdkit
On Fri, Dec 09, 2022 at 11:17:22AM -0600, Jonathon Jongsma wrote:
> On 12/9/22 4:09 AM, Peter Krempa wrote:
> >On Thu, Oct 20, 2022 at 16:59:00 -0500, Jonathon Jongsma wrote:
> >>Add some helper functions to build a virCommand object and run the
> >>nbdkit process for a given virStorageSource.
> >>
> >>Signed-off-by: Jonathon Jongsma
> >>---
> >> src/qemu/qemu_nbdkit.c | 251 +
> >> src/qemu/qemu_nbdkit.h | 10 ++
> >> 2 files changed, 261 insertions(+)
> >>
>
> ...
>
>
> >>+static int
> >>+qemuNbdkitProcessBuildCommandSSH(qemuNbdkitProcess *proc,
> >>+ virCommand *cmd)
> >>+{
> >>+const char *user = NULL;
> >>+virStorageNetHostDef *host = >source->hosts[0];
> >>+g_autofree char *portstr = g_strdup_printf("%u", host->port);
> >>+
> >>+/* nbdkit plugin name */
> >>+virCommandAddArg(cmd, "ssh");
> >>+
> >>+virCommandAddArgPair(cmd, "host", host->name);
> >>+virCommandAddArgPair(cmd, "port", portstr);
> >>+virCommandAddArgPair(cmd, "path", proc->source->path);
> >>+
> >>+if (proc->source->auth)
> >>+user = proc->source->auth->username;
> >>+else if (proc->source->ssh_user)
> >>+user = proc->source->ssh_user;
> >
> >So there's a bit problem with this, which also explains my reluctance to
> >simply supporting the SSH protocol in the current state without properly
> >designing other required parameters for SSH authentication. And even
> >then it will most likely break "historical compatibility".
> >
> >So historically we didn't implement ssh protocol at all. Users (notably
> >libguestfs) worked this around by adding a wrapper QCOW2 image and
> >setting up the backing store string such that qemu would access the ssh
> >image.
> >
> >When blockdev support was added I needed a way to port the existing
> >functionality specifically for libguestfs. This made me add the ssh
> >protocol and forwart the minimum set of properties to the image (thus
> >the 'ssh_user' field.
Off topic, but I wanted to check what we do now. We generate this XML
fragment:
$ guestfish --format=raw -a ssh://foo/var/tmp/fedora-36.img -i -vx
...
which I think is correct(?)
> >But there's another thing that was always configured out-of-band (via
> >environment variables) and that is the ssh agent socket path or ssh key
> >path.
Right - this doesn't work in fact:
Original error from libvirt: internal error: process exited while connecting to
monitor: 2022-12-09T18:16:45.674960Z qemu-kvm: -blockdev
{"driver":"ssh","path":"/var/tmp/fedora-36.img","server":{"host":"foo","port":"22"},"node-name":"libvirt-2-storage","cache":{"direct":false,"no-flush":false},"auto-read-only":true,"discard":"unmap"}:
failed to authenticate using publickey authentication and the identities held
by your ssh-agent [code=1 int1=-1]
Am I doing something wrong here? I have to confess it's been a very
long time since I tried to use the ssh support directly in libguestfs
(see below for why). So I don't quite remember how it's supposed to
work, or else it broke and I didn't notice.
> >These are not present in the virStorageSource because they are not even
> >configured for the blockdev 'ssh' protocol driver directly.
> >
> >Now with switching to nbdkit this means agent/sshkey authentication will
> >necessarily break for such usage as we simply don't have the
> >information.
> >
> >So what to do about it?
> >
> >We can either break the old way completely, which I guess would be
> >mostly okay since libguestfs most likely now uses nbdkit anyways, and
> >then design it properly. Obviously since the qemu ssh blockdev backend
> >driver still configures stuff using environment variables thus the new
> >configuration will be available only when nbdkit is in use.
So for libguestfs as above we're using libvirt (and hence indirectly
qemu's ssh). That's broken, but no one reported the bug so I guess
it's little used.
But you're right that for virt-v2v we switched to using
nbdkit-ssh-plugin a while back and that really is used and tested
extensively. (In this case we use libvirt + qemu's nbd driver to talk
to the nbdkit endpoint.)
> >Other possibility, if we want to keep old functionality working as it
> >was, is to avoid the use of nbdkit.
> >
> >Either way the implementation of SSH as done in this series simply just
> >breaks SSH usage completely, by not being able to support the old hacky
> >way of using agent/sshkey, not implementing any new support and not even
> >implementing password authentication.
> >
> >What now? I don't really care too deeply about the hacky impl we have
> >now. If libguestfs is no longer using I reckon we can simply break it
> >and not deal with the fallout.
> >
> >I'm not sure how others care about that though.
> >
> >Obviously another option (besides doing a proper desing on
> >authentication config) is to not touch anything ssh-related for now.
> >
>
>
[PATCH] rpc: Pass OPENSSL_CONF through to ssh invocations
It's no longer possible for libvirt to connect over the ssh transport from RHEL 9 to RHEL 5. This is because SHA1 signatures have been effectively banned in RHEL 9 at the openssl level. They are required to check the RHEL 5 host key. Note this is a separate issue from openssh requiring additional configuration in order to connect to older servers. Connecting from a RHEL 9 client to RHEL 5 server: $ cat ~/.ssh/config Host 192.168.0.91 KexAlgorithms+diffie-hellman-group14-sha1 MACs +hmac-sha1 HostKeyAlgorithms+ssh-rsa PubkeyAcceptedKeyTypes +ssh-rsa PubkeyAcceptedAlgorithms +ssh-rsa $ virsh -c 'qemu+ssh://root@192.168.0.91/system' list error: failed to connect to the hypervisor error: Cannot recv data: ssh_dispatch_run_fatal: Connection to 192.168.0.91 port 22: error in libcrypto: Connection reset by peer "error in libcrypto: Connection reset by peer" is the characteristic error of openssl having been modified to disable SHA1 by default. (You will not see this on non-RHEL-derived distros.) You could enable the legacy crypto policy which downgrades security on the entire host, but a more fine-grained way to do this is to create an alternate openssl configuration file that enables the "forbidden" signatures. However this requires passing the OPENSSL_CONF environment variable through to ssh to specify the alternate configuration. Libvirt filters out this environment variable, but this commit allows it through. With this commit: $ cat /var/tmp/openssl.cnf .include /etc/ssl/openssl.cnf [openssl_init] alg_section = evp_properties [evp_properties] rh-allow-sha1-signatures = yes $ OPENSSL_CONF=/var/tmp/openssl.cnf ./run virsh -c 'qemu+ssh://root@192.168.0.91/system' list root@192.168.0.91's password: Id Name State Essentially my argument here is that OPENSSL_CONF is sufficiently similar in nature to KRB5CCNAME, SSH* and XAUTHORITY that we should permit it to be passed through. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2062360 Signed-off-by: Richard W.M. Jones --- src/rpc/virnetsocket.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 32f506d2d4..8280bda007 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -855,6 +855,7 @@ int virNetSocketNewConnectSSH(const char *nodename, virCommandAddEnvPass(cmd, "KRB5CCNAME"); virCommandAddEnvPass(cmd, "SSH_AUTH_SOCK"); virCommandAddEnvPass(cmd, "SSH_ASKPASS"); +virCommandAddEnvPass(cmd, "OPENSSL_CONF"); virCommandAddEnvPass(cmd, "DISPLAY"); virCommandAddEnvPass(cmd, "XAUTHORITY"); virCommandClearCaps(cmd); -- 2.31.1
Re: Network disks and replacing qemu-block-curl|ssh with nbdkit
On Wed, Apr 20, 2022 at 09:36:29AM +0200, Peter Krempa wrote: > I'll post patches to address that, but the question is whether we want > to bother with actually supporting the password authentication or not, > because the simpler approach to fixing the bug is to simply allow it. Did you mean: Simply _not_ allow it? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
Re: Network disks and replacing qemu-block-curl|ssh with nbdkit
On Tue, Apr 19, 2022 at 03:00:58PM -0500, Jonathon Jongsma wrote:
> On 4/19/22 12:31 PM, Richard W.M. Jones wrote:
> >On Tue, Apr 19, 2022 at 11:40:49AM -0500, Jonathon Jongsma wrote:
> >>And now I notice that we do not actually have support for 'ssh'
> >>network disks in our xml schema either [3]. The
> >>VIR_STORAGE_NET_PROTOCOL_SSH enum value was originally added with a
> >>comment that it allows using the ssh protocol in backing chains.
> >>I've also seen some commits indicating that some of the support for
> >>ssh disk sources may be related to libguestfs usage in some way.
> >>cc'ing Rich and Peter in case they can add any historical context
> >>here.
> >>
> >>[3]
> >>https://gitlab.com/libvirt/libvirt/-/blob/136b821f183deb0b58c571211f6917985bed3308/src/conf/schemas/domaincommon.rng#L2150
> >
> >Peter will remember this better, but IIRC the history was that
> >virt-v2v used ssh protocol in backing files when accessing
> >certain disks, ie:
> >
> >- qemu-img create -b ssh:blah overlay.qcow2
> >- use libvirt to open overlay.qcow2
> >
> >Originally libvirt didn't care about this detail, but later libvirt
> >was changed so that it validated the backing chain. Peter added
> >support for ssh in the backing chain.
> >
> >Now actual support for protocol='ssh' (as in, the main drive, not only
> >in the backing chain), while not currently documented, seems to be
> >sort of working. libguestfs will try to create a protocol='ssh' drive
> >through libvirt if you ask it:
> >
> > $ guestfish --format=raw -a ssh://foo/var/tmp/newdisk run
> > libguestfs: error: could not create appliance through libvirt.
> > ...
> > Original error from libvirt: internal error: qemu unexpectedly closed the
> > monitor: 2022-04-19T17:19:47.096384Z qemu-kvm: -blockdev
> > {"driver":"ssh","path":"/var/tmp/newdisk","server":{"host":"foo","port":"22"},"node-name":"libvirt-2-storage","cache":{"direct":false,"no-flush":false},"auto-read-only":true,"discard":"unmap"}:
> > failed to authenticate using publickey authentication and the identities
> > held by your ssh-agent [code=1 int1=-1]
> >
> >If you add -vx to the guestfish command you can see the libvirt XML
> >fragment that was used:
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >which looks plausible. I don't understand why it doesn't work,
> >because I've got my agent set up correctly. I wonder if it's not
> >passing SSH_AUTH_SOCK through to session virtqemud?
> >
> >Rich.
>
> This looks like the same thing:
> https://bugzilla.redhat.com/show_bug.cgi?id=1140166 (and associated
> duplicates). It was finally closed deferred last year.
>
> Does this mean that the ssh disk source never properly worked in libvirt?
It seems possible. That bug was closed after we switched virt-v2v to
using nbdkit-ssh-plugin in these commits:
https://github.com/libguestfs/virt-v2v/commit/8312c03dc0f688b8e99e5602658fa3e288e29156
https://github.com/libguestfs/virt-v2v/commit/10f8324bf8b97466092f9a43951a7766e0def947
https://github.com/libguestfs/virt-v2v/commit/d4f8e5a4a01b0fbbb0158d0acc11f1920a0ac4fd
https://github.com/libguestfs/virt-v2v/commit/7a6f6113a25f96c813d2e73ee7e6cbd1606cfe4b
Be nice to support ssh in libvirt if it's not too hard, since ssh/sftp
is probably the most widely available remote protocol after http.
Almost every server is running sshd.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
Re: Network disks and replacing qemu-block-curl|ssh with nbdkit
On Tue, Apr 19, 2022 at 11:40:49AM -0500, Jonathon Jongsma wrote:
> Well, As far as I can tell, there is no valid XML for exercising
> http auth. The schema for http(s) sources does not include any
> element [1]. And the schema for the element [2]
> requires a element with a required 'type' attribute, and
> the only choices for 'type' are 'ceph' or 'iscsi', neither of which
> apply to http authentication.
>
> So it looks to me like http auth was never supported, rather than a
> regression. It also seems that it would require some additional
> schema changes to support this.
>
>
> [1]
> https://gitlab.com/libvirt/libvirt/-/blob/136b821f183deb0b58c571211f6917985bed3308/src/conf/schemas/domaincommon.rng#L1974
>
> [2]https://gitlab.com/libvirt/libvirt/-/blob/136b821f183deb0b58c571211f6917985bed3308/src/conf/schemas/domaincommon.rng#L6969
As noted in my other reply, simple http auth is not practically very
useful for the kinds of sources we want to access, so if libvirt never
supported it I wouldn't bother adding that complexity now.
> >>3. blockdev-create
...
nbdkit support for creating SSH remote files is upstream now:
https://gitlab.com/nbdkit/nbdkit/-/commit/0793f30b1071753532362b2ebf9cb8156a88c3c3
> And now I notice that we do not actually have support for 'ssh'
> network disks in our xml schema either [3]. The
> VIR_STORAGE_NET_PROTOCOL_SSH enum value was originally added with a
> comment that it allows using the ssh protocol in backing chains.
> I've also seen some commits indicating that some of the support for
> ssh disk sources may be related to libguestfs usage in some way.
> cc'ing Rich and Peter in case they can add any historical context
> here.
>
> [3]
> https://gitlab.com/libvirt/libvirt/-/blob/136b821f183deb0b58c571211f6917985bed3308/src/conf/schemas/domaincommon.rng#L2150
Peter will remember this better, but IIRC the history was that
virt-v2v used ssh protocol in backing files when accessing
certain disks, ie:
- qemu-img create -b ssh:blah overlay.qcow2
- use libvirt to open overlay.qcow2
Originally libvirt didn't care about this detail, but later libvirt
was changed so that it validated the backing chain. Peter added
support for ssh in the backing chain.
Now actual support for protocol='ssh' (as in, the main drive, not only
in the backing chain), while not currently documented, seems to be
sort of working. libguestfs will try to create a protocol='ssh' drive
through libvirt if you ask it:
$ guestfish --format=raw -a ssh://foo/var/tmp/newdisk run
libguestfs: error: could not create appliance through libvirt.
...
Original error from libvirt: internal error: qemu unexpectedly closed the
monitor: 2022-04-19T17:19:47.096384Z qemu-kvm: -blockdev
{"driver":"ssh","path":"/var/tmp/newdisk","server":{"host":"foo","port":"22"},"node-name":"libvirt-2-storage","cache":{"direct":false,"no-flush":false},"auto-read-only":true,"discard":"unmap"}:
failed to authenticate using publickey authentication and the identities held
by your ssh-agent [code=1 int1=-1]
If you add -vx to the guestfish command you can see the libvirt XML
fragment that was used:
which looks plausible. I don't understand why it doesn't work,
because I've got my agent set up correctly. I wonder if it's not
passing SSH_AUTH_SOCK through to session virtqemud?
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org
Re: Network disks and replacing qemu-block-curl|ssh with nbdkit
On Fri, Apr 15, 2022 at 10:09:59AM +0100, Richard W.M. Jones wrote: > I agree we should implement creation for ssh disks (not sure if it's > possible or even makes sense for curl). Shouldn't be too difficult. https://listman.redhat.com/archives/libguestfs/2022-April/028680.html Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Re: Network disks and replacing qemu-block-curl|ssh with nbdkit
On Thu, Apr 14, 2022 at 05:02:46PM -0500, Jonathon Jongsma wrote: > 1. secrets [...] > Fortunately, nbdkit provides a method for reading cookies and > passwords from a file, which should be secure if the file has > permissions set properly. So I'm currently planning to write a file > containing the cookies and pass them to nbdkit by specifying the > filename. But I'm still confused about the username/password > possibility. You can also send the password or cookie over an inherited file descriptor, which has the possible advantage that the secret will never hit the disk at all. For completeness I should say that we found HTTP authentication against some servers to be quite slow (presumably because validating a password involves a lot of machinery so doing it on every request is slow). For those servers we implemented a complicated scheme where you could make an authenticated request, fetch the cookie that the server sends back, send back the cookie, _and_ autorenew the cookie if it times out. (Did I say this was complicated?) This is required for at least VMware servers and Docker registries. https://libguestfs.org/nbdkit-curl-plugin.1.html#HEADER-AND-COOKIE-SCRIPTS I wouldn't try implementing this through libvirt ... > 2. readahead > > 3. blockdev-create See also: https://listman.redhat.com/archives/libguestfs/2022-April/028674.html I agree we should implement creation for ssh disks (not sure if it's possible or even makes sense for curl). Shouldn't be too difficult. You might also want to think about VDDK disk support, ie. is it possible to make the nbdkit stuff generic enough that nbdkit-vddk-plugin can be slotted in later? It would allow a libvirt domain to be backed with remote disks stored on VMFS, or to use VMware's own proprietary drivers to open local VMDK files, both significant enhancements. 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
Re: [PATCH 0/1] vmx: Fix mapping
On Mon, Oct 04, 2021 at 04:50:51PM +0200, Laszlo Ersek wrote: > On 10/04/21 11:59, Richard W.M. Jones wrote: > > It turns out that changing the qemu implementation is painful, > > particularly if we wish to maintain backwards compatibility of the > > command line and live migration. > > > > Instead I opted to document comprehensively what all the > > different hypervisors do: > > > > > > https://github.com/libguestfs/virt-v2v/blob/master/docs/vm-generation-id-across-hypervisors.txt > > > Unfortunately QEMU made a significant mistake when implementing this > > feature. Because the string is 128 bits wrong, they decided it must > ^^ > > Haha, that's a great typo :) > > > be a UUID (as you can see above there is no evidence that Microsoft > > who wrote the original spec thought it was). Following from this > > incorrect assumption, they stated that the "UUID" must be supplied to > > qemu in big endian format and must be byteswapped when writing it to > > guest memory. Their documentation says that they only do this for > > little endian guests, but this is not true of their implementation > > which byte swaps it for all guests. > > I don't think this is what section "Endian-ness Considerations" in > "docs/specs/vmgenid.txt" says. That text says that the *device* uses > little-endian format. That's independent of the endianness of *CPU* of > the particular guest architecture. > > So the byte-swapping in the QEMU code occurs unconditionally because > QEMU's UUID type is inherently big endian, and the device model in > question is fixed little endian. The guest CPU's endianness is > irrelevant as far as the device is concerned. > > If a BE guest CPU intends to read the generation ID and to interpret it > as a set of integers, then the guest CPU is supposed to byte-swap the > appropriate fields itself. > > > References > > I suggest adding two links in this section, namely to: > - docs/specs/vmgenid.txt > - hw/acpi/vmgenid.c Fair enough - I've made the changes you suggest (same URL as above). Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Re: [PATCH 0/1] vmx: Fix mapping
It turns out that changing the qemu implementation is painful, particularly if we wish to maintain backwards compatibility of the command line and live migration. Instead I opted to document comprehensively what all the different hypervisors do: https://github.com/libguestfs/virt-v2v/blob/master/docs/vm-generation-id-across-hypervisors.txt On Thu, Sep 30, 2021 at 10:16:20AM +0100, Richard W.M. Jones wrote: > I was going to suggest something like: > > aa-bb-cc.. > or > aabbcc.. After thinking about this some more, the real implementation on Windows guest and host is two 64 bit little-endian integers[1]. How about implementing this exactly the same way as Hyper-V (and VMware): 0x8877665544332211 0x00ffeeddccbbaa99 This would have to be mapped to the following qemu command line: qemu -device vmgenid,guid=44332211-6655-8877-99aa-bbccddeeff00 which would unmangle to the following in guest physical memory: 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff 00 The equivalent back-compat option would be: 44332211-6655-8877-99aa-bbccddeeff00 Rich. [1] No one has ever thought what to do about big-endian guests. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
Re: [Libguestfs] translating CD-ROM device paths from i440fx to Q35 in virt-v2v (was: test-v2v-cdrom: update the CD-ROM's bus to SATA in the converted domain)
On Thu, Sep 30, 2021 at 12:53:51PM +0100, Richard W.M. Jones wrote: > On Thu, Sep 30, 2021 at 01:12:39PM +0200, Laszlo Ersek wrote: > > All this requires virt-v2v to parse complete elements from the > > original domain XML, and to generate complete elements in the > > destination domain XML. Is that feasible? Just to put a bit of flesh on these bones, links to the source: > The input is not always (in fact, hardly ever) full libvirt XML. It's > input specific to the hypervisor. For VMware it might be: > > - the *.vmx file (the real source of truth) (-i vmx) Here's the virt-v2v code that can parse either a local or remote (over ssh) *.vmx file into virt-v2v's internal metadata representation: https://github.com/libguestfs/virt-v2v/blob/master/input/parse_domain_from_vmx.ml Libvirt also has a vmx driver, but I decided not to use it because it just adds an extra translation step and probably loses fidelity in the process. In all cases, the *.vmx file is the source of truth for VMware. > - partial libvirt XML generated by libvirt's vpx driver, but this is >derived from information from VMware APIs and ultimately that comes >from the *.vmx file (-i libvirt -ic esx:// or -ic vpx://) The libvirt driver: https://gitlab.com/libvirt/libvirt/-/tree/master/src/esx I've hacked on this one a little bit in the past, and ... it's complicated. > - the *.ovf file (-i ova) OVF is a pseudo-standard invented by VMware, but essentially a plot so they can say they are "standard" when in fact it's just a data dump of internal VMware structures with no compatibility across software that generates or consumes OVF. Here's our OVF parser (for VMware-OVF): https://github.com/libguestfs/virt-v2v/blob/master/input/OVF.ml We can also generate OVF, but we generate the RHV-flavour of OVF which (see above) is not related except that some XML element names overlap. > - nothing at all! (-i disk) If you give virt-v2v just a disk then it will invent some metadata: https://github.com/libguestfs/virt-v2v/blob/5adf437bcc00586961d8aa058559f86eb5165149/input/input.ml#L235 Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Re: translating CD-ROM device paths from i440fx to Q35 in virt-v2v (was: test-v2v-cdrom: update the CD-ROM's bus to SATA in the converted domain)
On Thu, Sep 30, 2021 at 01:12:39PM +0200, Laszlo Ersek wrote: > All this requires virt-v2v to parse complete elements from the > original domain XML, and to generate complete elements in the > destination domain XML. Is that feasible? The input is not always (in fact, hardly ever) full libvirt XML. It's input specific to the hypervisor. For VMware it might be: - the *.vmx file (the real source of truth) (-i vmx) - partial libvirt XML generated by libvirt's vpx driver, but this is derived from information from VMware APIs and ultimately that comes from the *.vmx file (-i libvirt -ic esx:// or -ic vpx://) - the *.ovf file (-i ova) - nothing at all! (-i disk) Also we don't currently try to find or rewrite /dev/disk/ paths in guest configuration files. The only rewriting that happens is for /dev/[hs]d* block device filenames and a few others. The actual code that does this is convert/convert_linux.ml:remap_block_devices So I wouldn't over-think this. It's likely fine to identify such devices and rewrite them as "/dev/cdrom", assuming that (I didn't check) udev creates that symlink for any reasonably modern Linux. And if there's more than one attached CD to the source, only convert the first one and warn about but drop the others. Note that the aim of virt-v2v is to make it boot on the target, make sure the network works, and get the user to a login prompt. The MTV management app around virt-v2v allows site-specific pre- and post- configuration to happen (site-specific Ansible playbooks). There also exists ssh and remote console. For people using virt-v2v on the command line, if they get the guest to boot on the target and not every device fully works, there is an expectation that they can log in as root and fix small[*] things. Rich. [*] Obviously if the boot / system is completely broken, not that. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Re: [PATCH 0/1] vmx: Fix mapping
On Thu, Sep 30, 2021 at 09:47:01AM +0100, Daniel P. Berrangé wrote: > On Thu, Sep 30, 2021 at 08:33:48AM +0100, Richard W.M. Jones wrote: > > I propose we deprecate the guid parameter in: > > > > -device vmgenid,guid=8987940a-0951-2cc5-e815-10634ff550b9,id=vmgenid0 > > > > Instead it will be replaced by bytes= which will simply write > > the bytes, in the order they appear, into guest memory with no > > attempt to interpret or byte-swap. Something like: > > > > -device vmgenid,bytes=112233445566778899aabbccddeeff00,id=vmgenid0 > > > > (guid although deprecated will need to be kept around for a while, > > along with its weird byte-swapping behaviour). > > > > We will then have a plain and simple method to emulate the behaviour > > of other hypervisors. We will look at exactly what bytes they write > > to guest memory and copy that behaviour when v2v converting from those > > hypervisors. > > From the libvirt POV, I'm not expecting anything in QEMU to change > in this respect. If guid is replaced by a new attribute taking data > in a different way, then libvirt will have to remap itself, so that > existing usage in libvirt keeps working the same way as it did with > guid. Essentially from libvirt's POV, it is simply a documentation > issue to specify how the libvirt XML representation translates to > the guest visible representation, and ensure that all libvirt drivers > implement it the same way. The QEMU genid support arrived first so > that set the standard for how libvirt will represent it, that all > further libvirt hypervisor drivers need to match. I was going to suggest something like: aa-bb-cc.. or aabbcc.. with the type defaulting to guid for backwards compatibility. Does libvirt XML have any other fields were you're passing essentially small snippets of binary data? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Re: [PATCH 0/1] vmx: Fix mapping
More data: I found a colleague who has a Hyper-V instance with a Windows guest and he helped me to understand how Hyper-V represents generation IDs. Hyper-V has had support for generation IDs since long before Microsoft proposed the feature for standardization. Originally (I think pre-2013) Hyper-V used an XML description which included: 4a07df4361fdf4c883f97bc30e524b9d Note this is a pure hex string, not a GUID. In current Hyper-V, the same representation is used but it's embedded in a binary file. My colleague ran two Windows VMs on Hyper-V and examined the generation_id on the hypervisor and compared it to the output of VMGENID.EXE inside the guest. The first guest had this in the binary hypervisor metadata: 56b0 00 00 0e 67 65 6e 65 72 61 74 69 6f 6e 5f 69 64 |...generation_id| 56c0 00 40 00 00 00 38 00 30 00 35 00 61 00 32 00 38 |.@...8.0.5.a.2.8| 56d0 00 37 00 61 00 32 00 35 00 30 00 39 00 38 00 39 |.7.a.2.5.0.9.8.9| 56e0 00 65 00 34 00 66 00 65 00 36 00 66 00 36 00 39 |.e.4.f.e.6.f.6.9| 56f0 00 39 00 32 00 62 00 65 00 33 00 33 00 34 00 61 |.9.2.b.e.3.3.4.a| 5700 00 34 00 33 00 00 00 00 00 00 00 00 00 00 00 00 |.4.3| and reported the output of VMGENID in this guest as: VmCounterValue: fe6f6992be334a43:805a287a250989e4 The second guest had: 5360 c5 06 00 00 00 0e 67 65 6e 65 72 61 74 69 6f 6e |..generation| 5370 5f 69 64 00 40 00 00 00 65 00 62 00 66 00 62 00 |_id.@...e.b.f.b.| 5380 62 00 37 00 39 00 37 00 33 00 36 00 35 00 37 00 |b.7.9.7.3.6.5.7.| 5390 32 00 38 00 65 00 37 00 30 00 62 00 33 00 66 00 |2.8.e.7.0.b.3.f.| 53a0 64 00 33 00 63 00 37 00 31 00 33 00 65 00 63 00 |d.3.c.7.1.3.e.c.| 53b0 65 00 38 00 34 00 32 00 00 00 00 00 00 00 00 00 |e.8.4.2.| and VMGENID was: VmCounterValue: b3fd3c713ece842:ebfbb797365728e7 Note: - In both cases, the generation ID is clearly not a GUID. - The two 64 bit words are swapped over somewhere, but individual bytes are not byte-swapped, and there is again no evidence of UUID-aware byte swapping. -- So how do we get from where we are to a more sane vmgenid in qemu? I propose we deprecate the guid parameter in: -device vmgenid,guid=8987940a-0951-2cc5-e815-10634ff550b9,id=vmgenid0 Instead it will be replaced by bytes= which will simply write the bytes, in the order they appear, into guest memory with no attempt to interpret or byte-swap. Something like: -device vmgenid,bytes=112233445566778899aabbccddeeff00,id=vmgenid0 (guid although deprecated will need to be kept around for a while, along with its weird byte-swapping behaviour). We will then have a plain and simple method to emulate the behaviour of other hypervisors. We will look at exactly what bytes they write to guest memory and copy that behaviour when v2v converting from those hypervisors. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
Re: [PATCH 0/1] vmx: Fix mapping
On Wed, Sep 29, 2021 at 11:10:35AM +0100, Daniel P. Berrangé wrote: > On Wed, Sep 29, 2021 at 10:57:19AM +0100, Richard W.M. Jones wrote: > > Looking at the qemu code the problem IMHO is: > > > > https://gitlab.com/qemu-project/qemu/-/blob/6b54a31bf7b403672a798b6443b1930ae6c74dea/docs/specs/vmgenid.txt#L189 > > https://gitlab.com/qemu-project/qemu/-/blob/6b54a31bf7b403672a798b6443b1930ae6c74dea/hw/acpi/vmgenid.c#L37 > > > > This byte swapping makes no sense to me. How do we know that the > > guest is little endian? What will this code do for BE guests? I > > think qemu would be better off treating the "GUID" as a list of bytes > > and writing that exactly into the guest memory. > > This is an artifact of the HyperV only caring about x86 and thus leaving > endianness unspecified in the spec for GenID. QEMU docs say > > > Endian-ness Considerations: > --- > > Although not specified in Microsoft's document, it is assumed that the > device is expected to use little-endian format. > > All GUID passed in via command line or monitor are treated as big-endian. > GUID values displayed via monitor are shown in big-endian format. > > > So by extension if libvirt is passing the value from its XML straight > to QEMU, then libvirt has effectively defined that the XML is storing > it big-endian too. > > This could be where the confusion with VMX config is coming into play, > though the byte re-ordering in v2v seems more complex than just an > endianess-fiddle ? qemu's qemu_uuid_bswap function only swaps some of the fields. I think even more that applying qemu_uuid_bswap to these (not really) "UUIDs" is a nonsense. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
Re: [PATCH 0/1] vmx: Fix mapping
On Wed, Sep 29, 2021 at 10:46:38AM +0100, Richard W.M. Jones wrote: > I don't know why we decided to use a GUID for this. The feature > itself (https://go.microsoft.com/fwlink/?LinkId=260709) defines it as > an 128 bit / 8 byte number. The only connection to GUIDs is the size. *cough* .. 16 bytes :-) Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
Re: [PATCH 0/1] vmx: Fix mapping
On Wed, Sep 29, 2021 at 11:07:30AM +0100, Daniel P. Berrangé wrote: > I'm not sure if we actually need the full driver or not for testing > purposes. The the GenID is just in memory somewhere, and the somewhere > is reported via ACPI table entry. For QEMU its easy as the data is > exposed via fw_cfg which can be read from sysfs directly without > even needing to look at ACPI entries to find it. Not sure how we > find it with VMWare/HyperV though. This still has the problem that qemu is mangling the vmgenid. Nevertheless, on qemu-6.1.0-5.fc36.x86_64 I added this to a Linux guest: 11223344-5566-7788-99aa-bbccddeeff00 which turned into: -device vmgenid,guid=11223344-5566-7788-99aa-bbccddeeff00,id=vmgenid0 Inside the guest: # ls /sys/firmware/qemu_fw_cfg/by_name/etc/vmgenid_guid/ -l total 0 -r. 1 root root 4096 Sep 29 11:16 key -r. 1 root root 4096 Sep 29 11:16 name -r. 1 root root0 Sep 29 11:16 raw -r. 1 root root 4096 Sep 29 11:16 size # hexdump -C /sys/firmware/qemu_fw_cfg/by_name/etc/vmgenid_guid/raw 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 || * 0020 00 00 00 00 00 00 00 00 44 33 22 11 66 55 88 77 |D3".fU.w| 0030 99 aa bb cc dd ee ff 00 00 00 00 00 00 00 00 00 || 0040 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 || * 1000 But I think what I really need to do is look at the raw physical address: # hexdump -C /sys/firmware/qemu_fw_cfg/by_name/etc/vmgenid_addr/raw 28 f0 ff 7f 00 00 00 00 |(...| 0008 # dd if=/dev/mem bs=1 skip=$(( 0x7028 )) count=16 | hexdump -C 16+0 records in 16+0 records out 16 bytes copied, 6.0392e-05 s, 265 kB/s 44 33 22 11 66 55 88 77 99 aa bb cc dd ee ff 00 |D3".fU.w| 0010 I think for VMware I'm really going to need the kernel driver, unless there's some way that iasl can be used to extract the information? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Re: [PATCH 0/1] vmx: Fix mapping
Looking at the qemu code the problem IMHO is: https://gitlab.com/qemu-project/qemu/-/blob/6b54a31bf7b403672a798b6443b1930ae6c74dea/docs/specs/vmgenid.txt#L189 https://gitlab.com/qemu-project/qemu/-/blob/6b54a31bf7b403672a798b6443b1930ae6c74dea/hw/acpi/vmgenid.c#L37 This byte swapping makes no sense to me. How do we know that the guest is little endian? What will this code do for BE guests? I think qemu would be better off treating the "GUID" as a list of bytes and writing that exactly into the guest memory. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Re: [PATCH 0/1] vmx: Fix mapping
On Wed, Sep 29, 2021 at 10:33:43AM +0100, Daniel P. Berrangé wrote:
> On Wed, Sep 29, 2021 at 10:20:44AM +0100, Richard W.M. Jones wrote:
> > On Wed, Sep 29, 2021 at 10:01:55AM +0200, Michal Privoznik wrote:
> > > Apparently, parsing vmx.genid is not as easy as I thought. Anyway, it
> > > was brought up in a private thread that libvirt doesn't report correct
> > > UUIDs. For instance for the following input:
> > >
> > > vm.genid = "-8536691797830587195"
> > > vm.genidX = "-1723453263670062919"
> >
> > (The two lines above come from a VMware .vmx file)
> >
> > The only thing that really matters is what the guest sees. I ran
> > VMGENID.EXE (https://bugzilla.redhat.com/show_bug.cgi?id=1598350#c3
> > https://docs.microsoft.com/en-gb/windows/win32/hyperv_v2/virtual-machine-generation-identifier)
> > inside the guest and it showed:
> >
> > 8987940a09512cc5:e81510634ff550b9
> >
> > Note these numbers are the hex equivalents of the VMware .vmx numbers:
> >
> > >>> print("%x" % (2**64-8536691797830587195))
> > 8987940a09512cc5
> > >>> print("%x" % (2**64-1723453263670062919))
> > e81510634ff550b9
> >
> > > Libvirt would report:
> > >
> > > 8987940a-0951-2cc5-e815-10634ff550b9
> > >
> > > while virt-v2v would report:
> > >
> > > 09512cc5-940a-8987-b950-f54f631015e8
> >
> > After thinking about this a bit more, IMHO the real problem is
> > with qemu. If you pass this to qemu:
> >
> > -device vmgenid,guid=8987940a-0951-2cc5-e815-10634ff550b9,id=vmgenid0
> >
> > then inside the guest VMGENID shows 2cc509518987940a:b950f54f631015e8
> > (wrong)
> >
> > If you pass this to qemu:
> >
> > ...guid=09512cc5-940a-8987-b950-f54f631015e8...
> >
> > then inside the guest it sees 8987940a09512cc5:e81510634ff550b9
> > (the correct values, matching VMware).
> >
> > This is the reason why virt-v2v mangles the GUID, in order to trick
> > libvirt into passing a mangled GUID which gets mangled again by qemu
> > which is the same as unmangling it.
> >
> > So another way to fix this could be for us to fix qemu. We could just
> > pass the two numbers to qemu instead of using GUIDs, eg:
> >
> > -device vmgenid,low=0x8987940a09512cc5,high=0xe81510634ff550b9,id=vmgenid0
> >
> > and then we'd fix the implementation in qemu so that the low/high
> > values match what is seen by VMGENID.EXE in the guest.
> >
> > Or we could have libvirt use the current GUID in but to do the
> > mangling when it gets passed through to qemu (instead of virt-v2v
> > doing the mangling).
>
> On the libvirt side, the #1 most important thing is that a given
> XML string
>
> 8987940a-0951-2cc5-e815-10634ff550b9
>
> results in the same value being exposed to the guest OS, for both
> the QEMU and VMWare drivers. Whehter the guest sees the bytes in
> the same order is not essential, but it would be less of a surprise
> if it did match.
I don't know why we decided to use a GUID for this. The feature
itself (https://go.microsoft.com/fwlink/?LinkId=260709) defines it as
an 128 bit / 8 byte number. The only connection to GUIDs is the size.
> Ultimately as long as the mapping from libvirt XML to guest visible
> string is consistent across drivers, that's sufficient.
>
> > Adding qemu-devel because I'm interesting to see if the qemu
> > developers would prefer to fix this properly in qemu.
>
> No matter what order QEMU accepts the data in, it can be said to be
> functionally correct. If the order is "unusual" though, it ought to
> at least be documented how the QEMU order corresponds to guest visible
> order.
>
> Incidentally I wonder how much to trust VMGENID.EXE and whether that
> actally reports what it gets out of guest memory "as is", or whether
> it has done any byte re-ordering.
>
> I'm curious what Linux kernel sees for the genid on Vmware vs KVM
> hosts, as probably I'd trust that data more ?
As far as I can tell no driver has successfully made it upstream,
although there have been a few attempts:
https://lkml.org/lkml/2018/3/1/498
Here's a more recent one from 10 months ago:
https://lore.kernel.org/linux-doc/3e05451b-a9cd-4719-99d0-72750a304...@amazon.com/
If I have time I'll patch a Linux kernel with the second patch and see
how it relates to the VMware and KVM parameters, but it probably won't
be today.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
Re: [PATCH 0/1] vmx: Fix mapping
On Wed, Sep 29, 2021 at 10:01:55AM +0200, Michal Privoznik wrote:
> Apparently, parsing vmx.genid is not as easy as I thought. Anyway, it
> was brought up in a private thread that libvirt doesn't report correct
> UUIDs. For instance for the following input:
>
> vm.genid = "-8536691797830587195"
> vm.genidX = "-1723453263670062919"
(The two lines above come from a VMware .vmx file)
The only thing that really matters is what the guest sees. I ran
VMGENID.EXE (https://bugzilla.redhat.com/show_bug.cgi?id=1598350#c3
https://docs.microsoft.com/en-gb/windows/win32/hyperv_v2/virtual-machine-generation-identifier)
inside the guest and it showed:
8987940a09512cc5:e81510634ff550b9
Note these numbers are the hex equivalents of the VMware .vmx numbers:
>>> print("%x" % (2**64-8536691797830587195))
8987940a09512cc5
>>> print("%x" % (2**64-1723453263670062919))
e81510634ff550b9
> Libvirt would report:
>
> 8987940a-0951-2cc5-e815-10634ff550b9
>
> while virt-v2v would report:
>
> 09512cc5-940a-8987-b950-f54f631015e8
After thinking about this a bit more, IMHO the real problem is
with qemu. If you pass this to qemu:
-device vmgenid,guid=8987940a-0951-2cc5-e815-10634ff550b9,id=vmgenid0
then inside the guest VMGENID shows 2cc509518987940a:b950f54f631015e8 (wrong)
If you pass this to qemu:
...guid=09512cc5-940a-8987-b950-f54f631015e8...
then inside the guest it sees 8987940a09512cc5:e81510634ff550b9
(the correct values, matching VMware).
This is the reason why virt-v2v mangles the GUID, in order to trick
libvirt into passing a mangled GUID which gets mangled again by qemu
which is the same as unmangling it.
So another way to fix this could be for us to fix qemu. We could just
pass the two numbers to qemu instead of using GUIDs, eg:
-device vmgenid,low=0x8987940a09512cc5,high=0xe81510634ff550b9,id=vmgenid0
and then we'd fix the implementation in qemu so that the low/high
values match what is seen by VMGENID.EXE in the guest.
Or we could have libvirt use the current GUID in but to do the
mangling when it gets passed through to qemu (instead of virt-v2v
doing the mangling).
Adding qemu-devel because I'm interesting to see if the qemu
developers would prefer to fix this properly in qemu.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
Re: [libvirt PATCH 0/4] qemu: replace -device sga with -M graphics=off
On Thu, Sep 09, 2021 at 12:25:12PM +0100, Daniel P. Berrangé wrote: > SeaBIOS >= 1.11 / QEMU >= 2.11 no longer requires the 'sga' > device for serial console output from the BIOS. It can be > done directly with graphics=off machine option. > > This appears to be live migration compatible, despite > changing the number of option ROMS loaded, though I need a > little more testing to fully confirm this. > > Daniel P. Berrangé (4): > qemu: prevent use of on non-x86 arches > qemu: tweak error message to be more general purpose > qemu: switch to use -M graphics=off instead of -device sga > qemu: stop probing for '-device sga' support > > src/qemu/qemu_capabilities.c | 3 +-- > src/qemu/qemu_capabilities.h | 2 +- > src/qemu/qemu_command.c | 25 --- > src/qemu/qemu_validate.c | 15 --- > .../caps_2.11.0.x86_64.xml| 1 - > .../caps_2.12.0.x86_64.xml| 1 - > .../caps_3.0.0.x86_64.xml | 1 - > .../caps_3.1.0.x86_64.xml | 1 - > .../caps_4.0.0.x86_64.xml | 1 - > .../caps_4.1.0.x86_64.xml | 1 - > .../caps_4.2.0.x86_64.xml | 1 - > .../caps_5.0.0.x86_64.xml | 1 - > .../caps_5.1.0.x86_64.xml | 1 - > .../caps_5.2.0.x86_64.xml | 1 - > .../caps_6.0.0.x86_64.xml | 1 - > .../caps_6.1.0.x86_64.xml | 1 - > tests/qemuxml2argvdata/bios.args | 3 +-- > tests/qemuxml2argvtest.c | 3 +-- > 18 files changed, 25 insertions(+), 38 deletions(-) I didn't test it, but the changes look reasonable so: Reviewed-by: Richard W.M. Jones Equivalent (non-)change to libguestfs is: https://github.com/libguestfs/libguestfs/commit/e14ff937422115e23094ca4cce80ec9fb01c10b3 Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Re: [libvirt PATCH 1/2] docs: virtiofs: move legacy docs to the bottom
On Thu, Sep 09, 2021 at 03:58:28PM +0100, Stefan Hajnoczi wrote: > A number of legacy issues make the virtiofs kbase article hard to > understand. Most users don't need to configure NUMA or a memory backend > other than memfd. Move that information to the bottom of the article so > the recommended syntax is most prominent. > > Suggested-by: Richard W.M. Jones > Signed-off-by: Stefan Hajnoczi This is much better. There are a couple of minor points below but in general: Reviewed-by: Richard W.M. Jones > +Optional parameters > +=== > + > +More optional elements can be specified > + > +:: > + > + > + > + > + > + I always think with XML it's better to be completely clear about the context around the element so that users know where to put these extra elements. For this reason I'd probably write this: + + +... + + + + + > +Externally-launched virtiofsd > += > + > +Libvirtd can also connect the ``vhost-user-fs`` device to a ``virtiofsd`` > +daemon launched outside of libvirtd. In that case socket permissions, > +the mount tag and all the virtiofsd options are out of libvirtd's > +control and need to be set by the application running virtiofsd. > + > +:: > + > + Is there an extra '/' above ^^ ? I see the same mistake is present in the current page. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
Re: [PATCH] vmx: Parse vm.genid
On Mon, Aug 02, 2021 at 01:14:52PM +0100, Daniel P. Berrangé wrote: > On Mon, Aug 02, 2021 at 12:26:56PM +0100, Richard W.M. Jones wrote: > > On Mon, Aug 02, 2021 at 12:12:08PM +0100, Daniel P. Berrangé wrote: > > > On Mon, Aug 02, 2021 at 12:04:49PM +0100, Richard W.M. Jones wrote: > > > > On Mon, Aug 02, 2021 at 01:00:15PM +0200, Michal Prívozník wrote: > > > > > On 7/30/21 2:02 PM, Richard W.M. Jones wrote: > > > > > > On Thu, Jul 29, 2021 at 10:30:30AM +0200, Michal Privoznik wrote: > > > > > >> The VMware metadata file contains genid but we are not parsing > > > > > >> and thus reporting it in domain XML. However, it's not as > > > > > >> straightforward as one might think. The UUID reported by VMware > > > > > >> is not in its usual string form, but split into two signed long > > > > > >> longs. That means, we have to do a bit of trickery when parsing. > > > > > >> But looking around it's the same magic that libguestfs does: > > > > > >> > > > > > >> https://github.com/libguestfs/virt-v2v/blob/master/v2v/input_vmx.ml#L421 > > > > > >> > > > > > >> It's also explained by Rich on qemu-devel: > > > > > >> > > > > > >> https://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg02019.html > > > > > >> > > > > > >> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1598348 > > > > > >> Signed-off-by: Michal Privoznik > > > > > >> --- > > > > > >> > > > > > >> I've successfully ran vmx2xmltest on an s390x machine which means > > > > > >> that > > > > > >> there shouldn't be any endiandness problem. > > > > > >> > > > > > >> src/vmx/vmx.c | 30 > > > > > >> +++ > > > > > >> .../vmx2xml-esx-in-the-wild-10.xml| 1 + > > > > > >> 2 files changed, 31 insertions(+) > > > > > >> > > > > > > > > > > > > > > > > > Looked reasonable and seems to match the description here: > > > > > > > > > > > > https://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg02019.html > > > > > > > > > > > > Reviewed-by: Richard W.M. Jones > > > > > > > > > > Pushed, thanks. > > > > > > > > > > > > > > > > > Out of interest, what is this being consumed by? I will add this to > > > > > > virt-v2v when it goes upstream. > > > > > > > > > > I don't recall all the specifics (it was John who implemented it), but > > > > > IIRC it was needed for Windows guests. Something about identifying > > > > > them > > > > > uniquely. John? > > > > > > > > Sure, I understand what it's used for. I was just wondering if there > > > > are other consumers who want to pull the genID from VMware VMX files > > > > using libvirt. Seems like something quite specific to V2V scenarios. > > > > > > Could there even be a a case to be made for V2V to *not* preserve the > > > the genID value. eg If you see a genID in the existing config, then > > > write a /different/ genID value in the new VM, to indicate that this > > > new VM is a fork of the original VM ? > > > > https://images5.alphacoders.com/405/405572.jpg > > > > N! Successfully converted VMs are definitely not forks and > > shouldn't be used that way. > > Are we sure it doesn't end up that way indirectly. eg is there any > liklihood that people will do this sequence: > > 1. Stop original guest > > 2. Run virt-v2v > > 3. Start converted guest > > 4. Find something not right > > 5. Stop converted guest > > 6. Start original guest > > 7. Fix > > 8. Goto (1) > > > Step 5/6 here is effectively akin to rolling back to a saved snapshot. > Thus genID ought to change if this is something users are liable todo. We try to set things up so this never happens. While it might be fine for people trying stuff out, in production this could be a disaster if it's an unintentional roll-back. Anyhow that would be a management layer decision, virt-v2v itself preserves the VMgenID when possible. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Re: [PATCH] vmx: Parse vm.genid
On Mon, Aug 02, 2021 at 12:12:08PM +0100, Daniel P. Berrangé wrote: > On Mon, Aug 02, 2021 at 12:04:49PM +0100, Richard W.M. Jones wrote: > > On Mon, Aug 02, 2021 at 01:00:15PM +0200, Michal Prívozník wrote: > > > On 7/30/21 2:02 PM, Richard W.M. Jones wrote: > > > > On Thu, Jul 29, 2021 at 10:30:30AM +0200, Michal Privoznik wrote: > > > >> The VMware metadata file contains genid but we are not parsing > > > >> and thus reporting it in domain XML. However, it's not as > > > >> straightforward as one might think. The UUID reported by VMware > > > >> is not in its usual string form, but split into two signed long > > > >> longs. That means, we have to do a bit of trickery when parsing. > > > >> But looking around it's the same magic that libguestfs does: > > > >> > > > >> https://github.com/libguestfs/virt-v2v/blob/master/v2v/input_vmx.ml#L421 > > > >> > > > >> It's also explained by Rich on qemu-devel: > > > >> > > > >> https://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg02019.html > > > >> > > > >> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1598348 > > > >> Signed-off-by: Michal Privoznik > > > >> --- > > > >> > > > >> I've successfully ran vmx2xmltest on an s390x machine which means that > > > >> there shouldn't be any endiandness problem. > > > >> > > > >> src/vmx/vmx.c | 30 +++++++ > > > >> .../vmx2xml-esx-in-the-wild-10.xml| 1 + > > > >> 2 files changed, 31 insertions(+) > > > >> > > > > > > > > > > > Looked reasonable and seems to match the description here: > > > > > > > > https://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg02019.html > > > > > > > > Reviewed-by: Richard W.M. Jones > > > > > > Pushed, thanks. > > > > > > > > > > > Out of interest, what is this being consumed by? I will add this to > > > > virt-v2v when it goes upstream. > > > > > > I don't recall all the specifics (it was John who implemented it), but > > > IIRC it was needed for Windows guests. Something about identifying them > > > uniquely. John? > > > > Sure, I understand what it's used for. I was just wondering if there > > are other consumers who want to pull the genID from VMware VMX files > > using libvirt. Seems like something quite specific to V2V scenarios. > > Could there even be a a case to be made for V2V to *not* preserve the > the genID value. eg If you see a genID in the existing config, then > write a /different/ genID value in the new VM, to indicate that this > new VM is a fork of the original VM ? https://images5.alphacoders.com/405/405572.jpg N! Successfully converted VMs are definitely not forks and shouldn't be used that way. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
Re: [PATCH] vmx: Parse vm.genid
On Mon, Aug 02, 2021 at 01:00:15PM +0200, Michal Prívozník wrote: > On 7/30/21 2:02 PM, Richard W.M. Jones wrote: > > On Thu, Jul 29, 2021 at 10:30:30AM +0200, Michal Privoznik wrote: > >> The VMware metadata file contains genid but we are not parsing > >> and thus reporting it in domain XML. However, it's not as > >> straightforward as one might think. The UUID reported by VMware > >> is not in its usual string form, but split into two signed long > >> longs. That means, we have to do a bit of trickery when parsing. > >> But looking around it's the same magic that libguestfs does: > >> > >> https://github.com/libguestfs/virt-v2v/blob/master/v2v/input_vmx.ml#L421 > >> > >> It's also explained by Rich on qemu-devel: > >> > >> https://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg02019.html > >> > >> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1598348 > >> Signed-off-by: Michal Privoznik > >> --- > >> > >> I've successfully ran vmx2xmltest on an s390x machine which means that > >> there shouldn't be any endiandness problem. > >> > >> src/vmx/vmx.c | 30 +++ > >> .../vmx2xml-esx-in-the-wild-10.xml| 1 + > >> 2 files changed, 31 insertions(+) > >> > > > > > Looked reasonable and seems to match the description here: > > > > https://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg02019.html > > > > Reviewed-by: Richard W.M. Jones > > Pushed, thanks. > > > > > Out of interest, what is this being consumed by? I will add this to > > virt-v2v when it goes upstream. > > I don't recall all the specifics (it was John who implemented it), but > IIRC it was needed for Windows guests. Something about identifying them > uniquely. John? Sure, I understand what it's used for. I was just wondering if there are other consumers who want to pull the genID from VMware VMX files using libvirt. Seems like something quite specific to V2V scenarios. > Here's the commit that implemented it in libvirt: > > https://gitlab.com/libvirt/libvirt/-/commit/b50efe97ad1357f9dff26450daf68a7a53201bea Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
Re: [PATCH] vmx: Parse vm.genid
On Thu, Jul 29, 2021 at 10:30:30AM +0200, Michal Privoznik wrote:
> The VMware metadata file contains genid but we are not parsing
> and thus reporting it in domain XML. However, it's not as
> straightforward as one might think. The UUID reported by VMware
> is not in its usual string form, but split into two signed long
> longs. That means, we have to do a bit of trickery when parsing.
> But looking around it's the same magic that libguestfs does:
>
> https://github.com/libguestfs/virt-v2v/blob/master/v2v/input_vmx.ml#L421
>
> It's also explained by Rich on qemu-devel:
>
> https://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg02019.html
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1598348
> Signed-off-by: Michal Privoznik
> ---
>
> I've successfully ran vmx2xmltest on an s390x machine which means that
> there shouldn't be any endiandness problem.
>
> src/vmx/vmx.c | 30 +++
> .../vmx2xml-esx-in-the-wild-10.xml| 1 +
> 2 files changed, 31 insertions(+)
>
> diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
> index 1cd5a82227..04eabff18a 100644
> --- a/src/vmx/vmx.c
> +++ b/src/vmx/vmx.c
> @@ -1337,6 +1337,32 @@ virVMXConfigScanResultsCollector(const char* name,
> }
>
>
> +static int
> +virVMXParseGenID(virConf *conf,
> + virDomainDef *def)
> +{
> +long long vmid[2] = { 0 };
> +g_autofree char *uuidstr = NULL;
> +
> +if (virVMXGetConfigLong(conf, "vm.genid", [0], 0, true) < 0 ||
> +virVMXGetConfigLong(conf, "vm.genidX", [1], 0, true) < 0)
> +return -1;
> +
> +if (vmid[0] == 0 && vmid[1] == 0)
> +return 0;
> +
> +uuidstr = g_strdup_printf("%.16llx%.16llx", vmid[0], vmid[1]);
> +if (virUUIDParse(uuidstr, def->genid) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not parse UUID from string '%s'"), uuidstr);
> +return -1;
> +}
> +def->genidRequested = true;
> +
> +return 0;
> +}
> +
> +
>
> /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> *
> * VMX -> Domain XML
> @@ -1466,6 +1492,10 @@ virVMXParseConfig(virVMXContext *ctx,
> }
> }
>
> +/* vmx:vm.genid + vm.genidX -> def:genid */
> +if (virVMXParseGenID(conf, def) < 0)
> +goto cleanup;
> +
> /* vmx:annotation -> def:description */
> if (virVMXGetConfigString(conf, "annotation", >description,
>true) < 0) {
> diff --git a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-10.xml
> b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-10.xml
> index b8c522af1f..47ed637920 100644
> --- a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-10.xml
> +++ b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-10.xml
> @@ -1,6 +1,7 @@
>
>w2019biosvmware
>421a6177-5aa9-abb7-5924-fc376c18a1b4
> + 13c67c91-9f47-526f-b0d6-e4dd2e4bb4f9
>4194304
>4194304
>2
Looked reasonable and seems to match the description here:
https://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg02019.html
Reviewed-by: Richard W.M. Jones
Out of interest, what is this being consumed by? I will add this to
virt-v2v when it goes upstream.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
Re: Turning off -Werror
On Wed, Jul 28, 2021 at 11:31:13AM +0100, Daniel P. Berrangé wrote: > On Wed, Jul 28, 2021 at 11:20:27AM +0100, Richard W.M. Jones wrote: > > > > commit 3c3c55be66e230ef09ad927eda038dc32f01a166 > > Author: Daniel P. Berrangé > > Date: Thu Apr 8 11:50:30 2021 +0100 > > > > meson: don't probe for -Werror if --werror is enabled > > > > Builds are failing in Fedora Rawhide at the moment because of a > > warning being turned into an error. Fedora's spec file has this which > > is supposed to turn off -Werror, but it no longer works after the > > above commit was added. > > I don't believe that commit made a difference. Prior to that commit, > libvirt would always add Werror if biulding from git, even if meson > had already added Werror, causing a warning from meson. > > Now we only add Werror if building from git and meson has not already > added Werror itself. > > > https://src.fedoraproject.org/rpms/libvirt/blob/rawhide/f/libvirt.spec#_189 > > > > I'm trying to understand how you're supposed to turn off -Werror. > > According to meson documentation omitting --werror should work. > > According to some different docs, use --werror=false. Neither are > > working for me. > > Normally -Dwerror=false would be sufficient for building from the > tarball, but in the RPM we're using git to manage patches, so we > need both options: > >-Dwerror=false -Dgit_werror=disabled I made this change in Rawhide, so let's see how it goes ... https://src.fedoraproject.org/rpms/libvirt/c/7744acbb6bb8e88f28d3cbf7671741483964b95e?branch=rawhide https://koji.fedoraproject.org/koji/taskinfo?taskID=72834651 Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Turning off -Werror
commit 3c3c55be66e230ef09ad927eda038dc32f01a166 Author: Daniel P. Berrangé Date: Thu Apr 8 11:50:30 2021 +0100 meson: don't probe for -Werror if --werror is enabled Builds are failing in Fedora Rawhide at the moment because of a warning being turned into an error. Fedora's spec file has this which is supposed to turn off -Werror, but it no longer works after the above commit was added. https://src.fedoraproject.org/rpms/libvirt/blob/rawhide/f/libvirt.spec#_189 I'm trying to understand how you're supposed to turn off -Werror. According to meson documentation omitting --werror should work. According to some different docs, use --werror=false. Neither are working for me. I'm actually wondering if the commit above is wrong. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Re: [PATCH 0/2] qemu: block: Pass discard requests through the copy-on-read block filter (for 7.6?)
On Wed, Jul 28, 2021 at 09:51:37AM +0200, Peter Krempa wrote: > See 1/2 > > Peter Krempa (2): > qemu: block: Pass discard requests through the copy-on-read block > filter > NEWS: Mention fix for 'copy_on_read' disks with trimming enabled > > NEWS.rst| 6 ++ > src/qemu/qemu_block.c | 1 + > tests/qemuxml2argvdata/disk-copy_on_read.x86_64-latest.args | 2 +- > 3 files changed, 8 insertions(+), 1 deletion(-) Reviewed-by: Richard W.M. Jones Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
Re: [libvirt PATCH 5/5] qemu: wire up support for maximum CPU model
Patch series looks fine to me, and we can certainly use this feature. One question I have about it: Does this apply to all architectures equally, or will we need to detect it from capabilities? I mean to say, if this was added to libvirt version X, then I might mandate that you have to use libvirt >= X and not bother detecting it (as the feature is so useful). But if it applies only on some architectures then we'd have to detect it anyway. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
Re: [libvirt PATCH 2/5] conf: add reporting of "maximum" CPU mode in domain caps
On Tue, Feb 09, 2021 at 01:58:58PM +, Daniel P. Berrangé wrote: > +mode name='maximum' supported='yes' > + enum name='maximumMigratable' > +valueon/value > +valueoff/value > + /enum > +/mode > mode name='host-model' supported='yes' Hmmm - that "supported" attribute is something I hadn't noticed before. Does this mean is possible in qemu capabilities? A quick scan of the libguestfs code seems to show we don't cope with that. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Re: [PATCH 2/3] utils: Deprecate hex-with-suffix sizes
On Fri, Feb 05, 2021 at 01:25:18PM +0300, Vladimir Sementsov-Ogievskiy wrote: > 04.02.2021 22:07, Eric Blake wrote: > >Supporting '0x20M' looks odd, particularly since we have an 'E' suffix > > What about also deprecating 'E' suffix? (just my problem of reviewing > previous patch) Ha! What if people want to specify exabytes?! That actually works at the moment: $ nbdkit memory 7E --run 'qemu-io -f raw -c "r -v 1E 512" "$uri"' Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
Re: [PATCH v5 0/8] Configurable policy for handling deprecated interfaces
On Mon, Sep 21, 2020 at 02:54:15PM +0200, Peter Krempa wrote: > On Mon, Sep 21, 2020 at 13:45:14 +0100, Richard W.M. Jones wrote: > > Some general comments on using the patch: > > > > * For libguestfs I chose to add > > > > -compat deprecated-input=reject,deprecated-output=hide > > > > This is only enabled in developer builds of libguestfs when we > > are running qemu directly (not via libvirt). The patch for > > this is attached. > > > > * What's the point/difference in having reject vs crash? > > I'll be adding the following documentation for the qemu.conf entry in > libvirt controling the feature: > > +# The "reject" option is less harsh towards the VMs but some code paths > ignore > +# errors reported by qemu and thus it may not be obvious that a deprecated > +# command/field was used, thus it's suggested to use the "crash" option > instead. I'm not sure if libguestfs should use reject or crash. But since most of the benefit of this is going to be in detecting deprecated CLI parameters in future, reject should be sufficient for us. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Re: [PATCH v5 0/8] Configurable policy for handling deprecated interfaces
Some general comments on using the patch:
* For libguestfs I chose to add
-compat deprecated-input=reject,deprecated-output=hide
This is only enabled in developer builds of libguestfs when we
are running qemu directly (not via libvirt). The patch for
this is attached.
* What's the point/difference in having reject vs crash?
* I hope that by hiding deprecated QAPI fields we may detect
errors in libguestfs, but I suspect what'll happen is it'll
cause fall-back behaviour which might be harder to detect.
* Be *really* good to have this for command line parameters!
Notes on the attached patch:
* https://libguestfs.org/guestfs-building.1.html
* Simple test:
LIBGUESTFS_BACKEND=direct \
LIBGUESTFS_HV=~/d/qemu/build/x86_64-softmmu/qemu-system-x86_64 \
./run libguestfs-test-tool
* Run the full test suite:
LIBGUESTFS_HV=~/d/qemu/build/x86_64-softmmu/qemu-system-x86_64 \
make -k check-direct
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
>From 90df6dc8a3278800f9f9dc23f626df5fa00b5950 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones"
Date: Mon, 21 Sep 2020 13:18:05 +0100
Subject: [PATCH] lib: direct: Pass qemu -compat to detect deprecated features.
In developer versions of libguestfs only, pass the qemu -compat option
which will reject deprecated qemu features, giving us early warning if
we are using something that may be removed in future. This does not
affect stable branch builds or old versions of qemu which did not have
this flag.
---
lib/guestfs-internal.h | 3 +++
lib/launch-direct.c| 11 +++
2 files changed, 14 insertions(+)
diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h
index d7ec7215d..4ad1cd125 100644
--- a/lib/guestfs-internal.h
+++ b/lib/guestfs-internal.h
@@ -33,6 +33,9 @@
#include
+/* Is this a developer version of libguestfs? */
+#define IS_DEVELOPER_VERSION ((PACKAGE_VERSION_MINOR & 1) == 1)
+
/* Minimum required version of libvirt for the libvirt backend.
*
* This is also checked at runtime because you can dynamically link
diff --git a/lib/launch-direct.c b/lib/launch-direct.c
index b6ed9766f..3e42609ff 100644
--- a/lib/launch-direct.c
+++ b/lib/launch-direct.c
@@ -501,6 +501,17 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
if (guestfs_int_qemu_supports (g, data->qemu_data, "-enable-fips"))
flag ("-enable-fips");
+ /* In non-stable versions of libguestfs, pass the -compat option to
+ * qemu so we can look for potentially deprecated features.
+ */
+ if (IS_DEVELOPER_VERSION &&
+ guestfs_int_qemu_supports (g, data->qemu_data, "-compat")) {
+start_list ("-compat") {
+ append_list ("deprecated-input=reject");
+ append_list ("deprecated-output=hide");
+} end_list ();
+ }
+
/* Newer versions of qemu (from around 2009/12) changed the
* behaviour of monitors so that an implicit '-monitor stdio' is
* assumed if we are in -nographic mode and there is no other
--
2.28.0.rc2
Re: [ovirt-devel] [ARM64] Possiblity to support oVirt on ARM64
On Wed, Jul 22, 2020 at 04:29:15PM +0800, Zhenyu Zheng wrote: > Hi, > > Any other comments for this topic? libguestfs, nbdkit and virt-v2v have also worked (upstream) for years out of the box on aarch64, so there should be no problem there. If you're interested in RHV (ie. oVirt downstream on RHEL) then note we don't build virt-v2v for RHEL on aarch64. Virt-v2v is not very useful on aarch64 since there is not (yet) any public VMware version for aarch64, although VMware do have an internal build and rumoured plans for ESXi on aarch64. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Re: [PATCH 2/2] qemu_capabilities.c: drop 'kvm_pr' support for non-Power8 hosts
On Fri, Jun 19, 2020 at 06:04:33PM -0300, Daniel Henrique Barboza wrote:
> PPC64 has two KVM modules: kvm_hv and kvm_pr. The official supported
> module was always kvm_hv, while kvm_pr was used for internal testing
> or for very niche cases in Power 8 hosts, always without official
> IBM or distro support.
>
> Problem is, QMP will report KVM supportfor PPC64 if any of these
> modules is loaded in the host, and kvm_pr is broken in everything
> but Power8 (and will remain broken, since kvm_pr is unmaintained).
> This can lead to situations like [1], where the tooling is misled to
> believe that the host has KVM capabilities when in reality it
> doesn't.
>
> The first reaction would be to simply forsake kvm_pr support entirely
> and move on, but there is no reason for now to be disruptive with any
> Power8 guests in the wild that are using kvm_pr (somehow). A more
> subtle approach is to not claim QEMU_CAPS_KVM support in all cases
> that we know it's completely broken, allowing Power8 users to take
> their shot using kvm_pr in their VMs. We can remove kvm_pr support
> completely when the module is removed from the kernel.
>
> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1843865
I'm sorry I don't have a way to test this patch. However it looks
reasonable from here based on the description above and the changes to
the code below, so ACK.
Rich.
> CC: Leonardo Augusto Guimarães Garcia
> CC: Greg Kurz
> CC: David Gibson
> CC: Richard W.M. Jones
> Signed-off-by: Daniel Henrique Barboza
> ---
> src/qemu/qemu_capabilities.c | 38 ++--
> 1 file changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 484fff99e5..b1c1d4dd70 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -49,6 +49,7 @@
> #include "qemu_process.h"
> #include "qemu_firmware.h"
> #include "virutil.h"
> +#include "virkmod.h"
>
> #include
> #include
> @@ -3242,6 +3243,31 @@ virQEMUCapsProbeQMPTPM(virQEMUCapsPtr qemuCaps,
> }
>
>
> +static void
> +virQEMUCapsSetPPC64KVMState(virQEMUCapsPtr qemuCaps, virArch hostArch)
> +{
> +g_autoptr(virCPUDef) hostCPU = virCPUProbeHost(hostArch);
> +
> +/* At this moment, PPC64 has two KVM modules: kvm_hv and kvm_pr.
> + * QEMU will return KVM present and enabled = true if any of these
> + * is loaded in the host. Thing is, kvm_pr was never officially
> + * supported by IBM or any other distro, but people still ended
> + * up using it in Power8 hosts regardless. It is also currently
> + * unmaintained and broken on Power9, and will be sunset in the
> + * future. kvm_hv is the only KVM module that is and will be
> + * supported.
> + *
> + * Until then, do not claim QEMU_CAPS_KVM if there is only kvm_pr
> + * loaded in the host. There is a good chance that there are
> + * unsupported kvm_pr Power8 guests running in the wild, so let's
> + * cut some slack for this case, for now. */
> +if (STRNEQLEN(hostCPU->model, "POWER8", 6) && !virKModIsLoaded("kvm_hv"))
> +return;
> +
> +virQEMUCapsSet(qemuCaps, QEMU_CAPS_KVM);
> +}
> +
> +
> static int
> virQEMUCapsProbeQMPKVMState(virQEMUCapsPtr qemuCaps,
> qemuMonitorPtr mon)
> @@ -3252,8 +3278,16 @@ virQEMUCapsProbeQMPKVMState(virQEMUCapsPtr qemuCaps,
> if (qemuMonitorGetKVMState(mon, , ) < 0)
> return -1;
>
> -if (present && enabled)
> -virQEMUCapsSet(qemuCaps, QEMU_CAPS_KVM);
> +if (present && enabled) {
> +virArch hostArch = virArchFromHost();
> +
> +if (ARCH_IS_PPC64(hostArch)) {
> +virQEMUCapsSetPPC64KVMState(qemuCaps, hostArch);
> +return 0;
> +}
> +
> + virQEMUCapsSet(qemuCaps, QEMU_CAPS_KVM);
> +}
>
> return 0;
> }
> --
> 2.26.2
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org
Re: [PATCH ocaml 1/1] Add dune build system
e
> + (targets c_flags.sexp c_library_flags.sexp)
> + (deps (:discover discover.exe))
> + (action (run %{discover})))
> +
> +(rule
> + (targets libvirt_generated.c)
> + (deps (:generator generator.pl))
> + (action (run %{generator})))
> +
> +(rule
> + (targets libvirt_version.ml)
> + (deps (:version_in libvirt_version.ml.in))
> + (action (with-stdout-to %{targets}
> +(run sed
> "s/@PACKAGE_NAME@/ocaml-libvirt/;s/@PACKAGE_VERSION@/%{version:mllibvirt}/;"
> %{version_in}
> +
> +(alias
> + (name htmldoc)
> + (deps (universe)) ; let it run everytime
> + (action (progn
> +(run rm -rf html)
> +(run mkdir html)
> +(run ocamldoc -html -sort -colorize-code -d html
> %{dep:libvirt.mli} %{dep:libvirt_version.mli}
> diff --git a/libvirt/dune-project b/libvirt/dune-project
> new file mode 100644
> index 000..5ede662
> --- /dev/null
> +++ b/libvirt/dune-project
> @@ -0,0 +1,2 @@
> +(lang dune 1.0)
> +(version 0.6.1.5)
> diff --git a/libvirt/mllibvirt.opam b/libvirt/mllibvirt.opam
> new file mode 100644
> index 000..23eee04
> --- /dev/null
> +++ b/libvirt/mllibvirt.opam
> @@ -0,0 +1,33 @@
> +opam-version: "2.0"
> +name: "mllibvirt"
> +authors: "Richard W.M. Jones "
> +maintainer: "Richard W.M. Jones "
> +homepage: "https://libvirt.org/ocaml/;
> +dev-repo: "https://gitlab.com/libvirt/libvirt-ocaml.git;
> +bug-reports: "https://libvirt.org/bugs.html;
> +synopsis: "Libvirt bindings for OCaml"
> +description: """libvirt bindings for OCaml
> +
> +Libvirt is a C toolkit to interact with the virtualization capabilities
> +of recent versions of Linux (and other OSes)."""
> +license: "LGPL-2+"
> +tags: [
> + "libvirt"
> +]
> +build: [
> + ["dune" "build"]
> +]
> +depends: [
> + "conf-pkg-config" {build}
> + "ocamlfind" {build}
> +]
> +depexts: [
> + ["libvirt-dev"] {os-family = "debian"}
> + ["libvirt-devel"] {os-family = "rhel"}
> + ["libvirt-devel"] {os-family = "suse"}
> + ["libvirt-devel"] {os-distribution = "mageia"}
> + ["libvirt"] {os = "macos" & os-distribution = "homebrew"}
> + ["libvirt-dev"] {os-distribution = "alpine"}
> + ["libvirt"] {os-distribution = "nixos"}
> + ["libvirt"] {os-distribution = "arch"}
> +]
> --
> 2.25.2
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
FYI: Report about libvirt failing to compile on MSYS2 with an XDR error
.. is here: https://github.com/msys2/MINGW-packages/issues/6384 Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
Re: [PATCH] vmx: make 'fileName' optional for CD-ROMs
On Wed, Mar 18, 2020 at 01:44:18PM +0100, Pino Toscano wrote:
> (Apparently forgot to send it yesterday, so sending it with a small
> addedum.)
>
> On Tuesday, 17 March 2020 18:09:04 CET Richard W.M. Jones wrote:
> > My only worry about this patch is that it relies on fileName now
> > possibly being NULL, which means if there is any case that you've
> > missed now — or one added in future — which doesn't consider that
> > fileName might be NULL then it'll crash (libvirtd? or virsh? I'm not
> > sure).
>
> In case now (even in v2) fileName is used without checking, it will
> crash libvirt, as the esx/vmware drivers are built-in in the library.
>
> > I wonder if therefore it would be safer to set the string to a
> > known-good non-NULL value such as ‘strdup ("emptyBackingString")’?
>
> Thought about that, however "emptyBackingString" seems like a magic
> identifier, and it would be trading one hack with another, somehow.
Sure, I've no objections if you're happy with it, so you can
add my Reviewed-by tag to v2.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
Re: [PATCH v2 0/2] vmx: make 'fileName' optional for CD-ROMs
Works, so: Tested-by: Richard W.M. Jones I have no further comments about the patches themselves other than what I said on the first version. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Re: [PATCH] vmx: make 'fileName' optional for CD-ROMs
{
> ignore_value(virDomainDiskSetSource(*def, NULL));
> (*def)->startupPolicy = VIR_DOMAIN_STARTUP_POLICY_OPTIONAL;
> } else if (virDomainDiskSetSource(*def, fileName) < 0) {
> @@ -2325,7 +2326,7 @@ virVMXParseDisk(virVMXContext *ctx,
> virDomainXMLOptionPtr xmlopt, virConfPtr con
> }
> } else if (busType == VIR_DOMAIN_DISK_BUS_SCSI &&
> deviceType && STRCASEEQ(deviceType, "scsi-passthru")) {
> -if (STRPREFIX(fileName, "/vmfs/devices/cdrom/")) {
> +if (fileName && STRPREFIX(fileName, "/vmfs/devices/cdrom/")) {
> /* SCSI-passthru CD-ROMs actually are device='lun' */
> (*def)->device = VIR_DOMAIN_DISK_DEVICE_LUN;
> virDomainDiskSetType(*def, VIR_STORAGE_TYPE_BLOCK);
> @@ -2341,7 +2342,7 @@ virVMXParseDisk(virVMXContext *ctx,
> virDomainXMLOptionPtr xmlopt, virConfPtr con
> */
> goto ignore;
> }
> -} else if (STREQ(fileName, "emptyBackingString")) {
> +} else if (fileName && STREQ(fileName, "emptyBackingString")) {
> if (deviceType && STRCASENEQ(deviceType, "cdrom-image")) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Expecting VMX entry '%s' to be
> 'cdrom-image' "
> @@ -2355,7 +2356,8 @@ virVMXParseDisk(virVMXContext *ctx,
> virDomainXMLOptionPtr xmlopt, virConfPtr con
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Invalid or not yet handled value '%s' "
> "for VMX entry '%s' for device type '%s'"),
> - fileName, fileName_name,
> + fileName ? fileName : "(not present)",
> + fileName_name,
> deviceType ? deviceType : "unknown");
> goto cleanup;
> }
> diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx
> b/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx
> new file mode 100644
> index 00..36286cb20f
> --- /dev/null
> +++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx
> @@ -0,0 +1,4 @@
> +config.version = "8"
> +virtualHW.version = "4"
> +ide0:0.present = "true"
> +ide0:0.deviceType = "atapi-cdrom"
> diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml
> b/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml
> new file mode 100644
> index 00..af4a5ff9f6
> --- /dev/null
> +++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml
> @@ -0,0 +1,23 @@
> +
> + 0000-0000-0000--
> + 32768
> + 32768
> + 1
> +
> +hvm
> +
> +
> + destroy
> + restart
> + destroy
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c
> index 8d7b8ba2a4..1966aed6fe 100644
> --- a/tests/vmx2xmltest.c
> +++ b/tests/vmx2xmltest.c
> @@ -218,6 +218,7 @@ mymain(void)
> DO_TEST("cdrom-scsi-passthru", "cdrom-scsi-passthru");
> DO_TEST("cdrom-ide-file", "cdrom-ide-file");
> DO_TEST("cdrom-ide-empty", "cdrom-ide-empty");
> +DO_TEST("cdrom-ide-empty-2", "cdrom-ide-empty-2");
> DO_TEST("cdrom-ide-device", "cdrom-ide-device");
> DO_TEST("cdrom-ide-raw-device", "cdrom-ide-raw-device");
> DO_TEST("cdrom-ide-raw-auto-detect", "cdrom-ide-raw-auto-detect");
Firstly I can confirm that the patch does work, so:
Tested-by: Richard W.M. Jones
My only worry about this patch is that it relies on fileName now
possibly being NULL, which means if there is any case that you've
missed now — or one added in future — which doesn't consider that
fileName might be NULL then it'll crash (libvirtd? or virsh? I'm not
sure). I wonder if therefore it would be safer to set the string to a
known-good non-NULL value such as ‘strdup ("emptyBackingString")’?
Anyway as far as the patch goes it does seem fine so:
Reviewed-by: Richard W.M. Jones
Thanks for looking at this one.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
Re: [libvirt PATCH] src: fix mixup of stack and heap allocated data in auth callback
On Thu, Mar 05, 2020 at 04:54:10PM +, Daniel P. Berrangé wrote:
> In the following recent change:
>
> commit db72866310d1e520efa8ed2d4589bdb5e76a1c95
> Author: Daniel P. Berrangé
> Date: Tue Jan 14 10:40:52 2020 +
>
> util: add API for reading password from the console
>
> the fact that "bufptr" pointer may point to either heap or stack
> allocated data was overlooked. As a result, when the strdup was
> removed, we ended up returning a pointer to the local stack to
> the caller. When the caller referenced this stack pointer they
> got out garbage which fairly quickly resulted in a crash.
>
> Switching from fgets() to getline() lets to avoid the need for
> the stack allocated buffer entirely, which makes both cases
> of the switch consistent.
>
> Test case addition is inspired by the libguestfs test which
> caught this bug.
>
> Signed-off-by: Daniel P. Berrangé
> ---
> src/libvirt.c| 30 +++
> tests/Makefile.am| 2 ++
> tests/virsh-auth | 57
> tests/virsh-auth.xml | 5
> 4 files changed, 79 insertions(+), 15 deletions(-)
> create mode 100755 tests/virsh-auth
> create mode 100644 tests/virsh-auth.xml
>
> diff --git a/src/libvirt.c b/src/libvirt.c
> index a30eaa7590..cc9c2eb5a2 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -110,9 +110,8 @@ virConnectAuthCallbackDefault(virConnectCredentialPtr
> cred,
> size_t i;
>
> for (i = 0; i < ncred; i++) {
> -char buf[1024];
> -char *bufptr = buf;
> -size_t len;
> +char *buf = NULL;
> +size_t len = 0;
>
> switch (cred[i].type) {
> case VIR_CRED_EXTERNAL: {
> @@ -136,16 +135,17 @@ virConnectAuthCallbackDefault(virConnectCredentialPtr
> cred,
> if (fflush(stdout) != 0)
> return -1;
>
> -if (!fgets(buf, sizeof(buf), stdin)) {
> -if (feof(stdin)) { /* Treat EOF as "" */
> -buf[0] = '\0';
> -break;
> +if (getline(, , stdin) < 0) {
> +if (!feof(stdin)) {
> +return -1;
> }
> -return -1;
> +/* Use creddefault on EOF */
> +buf = NULL;
> +} else {
> +len = strlen(buf);
> +if (len != 0 && buf[len-1] == '\n')
> +buf[len-1] = '\0';
> }
> -len = strlen(buf);
> -if (len != 0 && buf[len-1] == '\n')
> -buf[len-1] = '\0';
> break;
>
> case VIR_CRED_PASSPHRASE:
> @@ -155,9 +155,9 @@ virConnectAuthCallbackDefault(virConnectCredentialPtr
> cred,
> if (fflush(stdout) != 0)
> return -1;
>
> -bufptr = virGetPassword();
> -if (STREQ(bufptr, ""))
> -VIR_FREE(bufptr);
> +buf = virGetPassword();
> +if (STREQ(buf, ""))
> +VIR_FREE(buf);
> break;
>
> default:
> @@ -165,7 +165,7 @@ virConnectAuthCallbackDefault(virConnectCredentialPtr
> cred,
> }
>
> if (cred[i].type != VIR_CRED_EXTERNAL) {
> -cred[i].result = bufptr ? bufptr : g_strdup(cred[i].defresult ?
> cred[i].defresult : "");
> +cred[i].result = buf ? buf : g_strdup(cred[i].defresult ?
> cred[i].defresult : "");
> cred[i].resultlen = strlen(cred[i].result);
> }
> }
It's clearly better to use getline here instead of fgets and
the large fixed-size stack buffer, so
ACK
Rich.
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 83326dbaa9..3b5abcc12b 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -164,6 +164,7 @@ EXTRA_DIST = \
> xlconfigdata \
> xmconfigdata \
> xml2vmxdata \
> + virsh-auth.xml \
> virstorageutildata \
> virfilecachedata \
> virresctrldata \
> @@ -406,6 +407,7 @@ test_scripts =
> libvirtd_test_scripts = \
> libvirtd-fail \
> libvirtd-pool \
> + virsh-auth \
> virsh-cpuset \
> virsh-define-dev-segfault \
> virsh-int-overflow \
> diff --git a/tests/virsh-auth b/tests/virsh-auth
> new file mode 100755
> index 00..d548694190
> --- /dev/null
> +++ b/tests/virsh-auth
> @@ -0,0 +1,57 @@
> +#!/usr/bin/env python3
> +# run virsh to validate interactive auth
> +
> +# Copyright (C) 2020 Red Hat, Inc.
> +
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation, either version 2 of the License, or
> +# (at your option) any later version.
> +
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU
virtlogd spinning on 100% CPU with the latest libvirt
Build libvirt from git (ccf7567329f). Using the libvirt ‘run’ script, run something like libguestfs-test-tool. I think basically any command which runs a guest will do. NB These commands are all run as NON-root: killall libvirtd lt-libvirtd virtlogd lt-virtlogd ./build/run libguestfs-test-tool Now there will be a lt-virtlogd process using 100% of CPU: PID USER PR NIVIRTRESSHR S %CPU %MEM TIME+ COMMAND 2572972 rjones20 0 47880 16256 14516 R 100.0 0.1 0:19.27 lt-virt+ $ ls /proc/2572972/fd -l total 0 lrwx--. 1 rjones rjones 64 Feb 17 17:45 0 -> /dev/null lrwx--. 1 rjones rjones 64 Feb 17 17:45 1 -> /dev/null lr-x--. 1 rjones rjones 64 Feb 17 17:45 11 -> /var/lib/sss/mc/passwd lr-x--. 1 rjones rjones 64 Feb 17 17:45 12 -> /var/lib/sss/mc/group lrwx--. 1 rjones rjones 64 Feb 17 17:45 13 -> 'socket:[4824]' lr-x--. 1 rjones rjones 64 Feb 17 17:45 14 -> 'pipe:[4825]' l-wx--. 1 rjones rjones 64 Feb 17 17:45 16 -> /home/rjones/.cache/libvirt/qemu/log/guestfs-xllxycje1blj4nmd.log l-wx--. 1 rjones rjones 64 Feb 17 17:45 17 -> /run/systemd/inhibit/1620.ref lrwx--. 1 rjones rjones 64 Feb 17 17:45 2 -> /dev/null lrwx--. 1 rjones rjones 64 Feb 17 17:45 3 -> 'socket:[48299981]' l-wx--. 1 rjones rjones 64 Feb 17 17:45 4 -> /run/user/1000/libvirt/virtlogd.pid lrwx--. 1 rjones rjones 64 Feb 17 17:45 5 -> 'socket:[48299984]' lrwx--. 1 rjones rjones 64 Feb 17 17:45 6 -> 'socket:[48299986]' lr-x--. 1 rjones rjones 64 Feb 17 17:45 7 -> 'pipe:[48299988]' l-wx--. 1 rjones rjones 64 Feb 17 17:45 8 -> 'pipe:[48299988]' lrwx--. 1 rjones rjones 64 Feb 17 17:45 9 -> 'anon_inode:[eventfd]' $ ls -ltr /home/rjones/.cache/libvirt/qemu/log/guestfs-xllxycje1blj4nmd.log -rw---. 1 rjones rjones 4003 Feb 17 17:44 /home/rjones/.cache/libvirt/qemu/log/guestfs-xllxycje1blj4nmd.log Only one thread running with this stack trace: Thread 1 (Thread 0x7fa51f219b40 (LWP 2572972)): #0 virObjectGetLockableObj (anyobj=0x55fb2896c200) at ../../src/util/virobject.c:393 #1 virObjectLock (anyobj=0x55fb2896c200) at ../../src/util/virobject.c:427 #2 0x7fa520fda48f in virNetServerHasClients (srv=0x55fb2896c200) at ../../src/rpc/virnetserver.c:966 #3 0x7fa520fd7b69 in daemonServerHasClients (payload=, key=, opaque=0x7ffc8adb5a47) at ../../src/rpc/virnetdaemon.c:916 #4 0x7fa520ea7140 in virHashForEach (data=, iter=, table=) at ../../src/util/virhash.c:639 #5 virHashForEach (table=0x55fb289571a0, iter=iter@entry=0x7fa520fd7b60 , data=data@entry=0x7ffc8adb5a47) at ../../src/util/virhash.c:627 #6 0x7fa520fd89ee in virNetDaemonHasClients (dmn=) at ../../src/rpc/virnetdaemon.c:927 #7 0x7fa520fd8aa5 in virNetDaemonRun (dmn=0x55fb28957110) at ../../src/rpc/virnetdaemon.c:842 #8 0x55fb27b5c8e9 in main (argc=, argv=0x7ffc8adb6188) at ../../src/logging/log_daemon.c:1153 pstack shows it's fairly busy and the stack trace is not very consistent, eg: #0 0x7fa52032aaed in g_free () at /lib64/libglib-2.0.so.0 #1 0x7fa520e73d1b in virFree (ptrptr=ptrptr@entry=0x55fb2896c1a8) at ../../src/util/viralloc.c:348 #2 0x7fa520e982d2 in virResetError (err=0x55fb2896c1a0) at ../../src/util/virerror.c:472 #3 virResetError (err=0x55fb2896c1a0) at ../../src/util/virerror.c:468 #4 0x7fa520e9963c in virEventRunDefaultImpl () at ../../src/util/virevent.c:341 #5 0x7fa520fd8abd in virNetDaemonRun (dmn=0x55fb28957110) at ../../src/rpc/virnetdaemon.c:858 #6 0x55fb27b5c8e9 in main (argc=, argv=0x7ffc8adb6188) at ../../src/logging/log_daemon.c:1153 #0 0x7fa520184a37 in poll () at /lib64/libc.so.6 #1 0x7fa520324e3e in g_main_context_iterate.isra () at /lib64/libglib-2.0.so.0 #2 0x7fa520324f73 in g_main_context_iteration () at /lib64/libglib-2.0.so.0 #3 0x7fa520e9a4f0 in virEventGLibRunOnce () at ../../src/util/vireventglib.c:496 #4 0x7fa520e99645 in virEventRunDefaultImpl () at ../../src/util/virevent.c:343 #5 0x7fa520fd8abd in virNetDaemonRun (dmn=0x55fb28957110) at ../../src/rpc/virnetdaemon.c:858 #6 0x55fb27b5c8e9 in main (argc=, argv=0x7ffc8adb6188) at ../../src/logging/log_daemon.c:1153 #0 0x7fa520322d9d in g_source_ref () at /lib64/libglib-2.0.so.0 #1 0x7fa520322e71 in g_source_iter_next () at /lib64/libglib-2.0.so.0 #2 0x7fa52032479f in g_main_context_check () at /lib64/libglib-2.0.so.0 #3 0x7fa520324de2 in g_main_context_iterate.isra () at /lib64/libglib-2.0.so.0 #4 0x7fa520324f73 in g_main_context_iteration () at /lib64/libglib-2.0.so.0 #5 0x7fa520e9a4f0 in virEventGLibRunOnce () at ../../src/util/vireventglib.c:496 #6 0x7fa520e99645 in virEventRunDefaultImpl () at ../../src/util/virevent.c:343 #7 0x7fa520fd8abd in virNetDaemonRun (dmn=0x55fb28957110) at ../../src/rpc/virnetdaemon.c:858 #8 0x55fb27b5c8e9 in main (argc=, argv=0x7ffc8adb6188) at ../../src/logging/log_daemon.c:1153 Rich.
Re: [PATCH v3 00/15] qemu: Handle 'size' and 'offset' attributes of 'raw' format
On Wed, Feb 12, 2020 at 07:03:11PM +0100, Peter Krempa wrote: > This series fixes and improves the 'json:' pseudo-protocol parser and > implements the 'offset' and 'size' attributes and exposes them as > in the XML. > > The previous version attempted an easy route, but that didn't cover all > cases. This version adds storage slice support for everything except > image creation. > > https://bugzilla.redhat.com/show_bug.cgi?id=1791788 This one seems to handle the ‘virt-v2v -i ova’ case, so it's good from my point of view. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Re: [PATCH 00/15] qemu: Handle 'size' and 'offset' attributes of 'raw' format
On Thu, Feb 06, 2020 at 08:51:52AM +0100, Peter Krempa wrote:
> This series fixes and improves the 'json:' pseudo-protocol parser and
> implements the 'offset' and 'size' attributes and exposes them as
> in the XML.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1791788
>
> Peter Krempa (15):
> virStorageSourceParseBackingJSON: Pass around original backing file
> string
> virStorageSourceParseBackingJSON: Move deflattening of json: URIs out
> of recursion
> virStorageSourceJSONDriverParser: annotate 'format' drivers
> virStorageSourceParseBackingJSON: Allow 'json:' pseudo URIs without
> 'file' wrapper
> virStorageSourceParseBackingJSON: Prevent arbitrary nesting with
> format drivers
> tests: virstorage: Add test cases for "json:" pseudo-URI without
> 'file' wrapper
> tests: virstorage: Add test data for json specified raw image with
> offset/size
> util: virstoragefile: Add data structure for storing storage source
> slices
> qemuBlockStorageSourceGetFormatRawProps: format 'offset' and 'size'
> for slice
> qemuDomainValidateStorageSource: Reject unsupported slices
> docs: formatdomain: Close on one of disk examples
> docs: Document the new sub-element of disk's
> conf: Implement support for of disk source
> tests: qemu: Add test data for the new element
> virStorageSourceParseBackingJSONRaw: Parse 'offset' and 'size'
> attributes
So with this patch, virt-v2v -i ova now fails with:
Original error from libvirt: unsupported configuration: format slice
is not supported for format 'vmdk' [code=67 int1=-1]
The overlay was created (by virt-v2v) with:
$ qemu-img create -q -f qcow2 -b 'json:{ "file": { "driver": "raw", "offset":
512, "size": 349405696, "file": { "driver": "file", "filename":
"/var/tmp/First.ova" } } }' -o 'compat=1.1,backing_fmt=vmdk' /tmp/v2vovl.qcow2
A simple test case to use is:
$ wget http://oirase.annexia.org/tmp/First.ova
$ virt-v2v -i ova First.ova -o null -v -x
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
Re: [PATCH ocaml 0/3] Small fixes for OCaml 4.10.
On Fri, Jan 24, 2020 at 04:13:14PM +, Daniel P. Berrangé wrote: > On Fri, Jan 24, 2020 at 03:01:51PM +, Daniel P. Berrangé wrote: > > On Fri, Jan 24, 2020 at 02:51:39PM +, Richard W.M. Jones wrote: > > > On Fri, Jan 24, 2020 at 02:35:01PM +, Daniel P. Berrangé wrote: > > > > On Sun, Jan 19, 2020 at 01:04:07PM +, Richard W.M. Jones wrote: > > > > > Basically OCaml 4.10 is much more strict about the difference between > > > > > string (immutable) and bytes (mutable). > > > > > > > > For all patches > > > > > > > > Reviewed-by: Daniel P. Berrangé > > > > > > > > I presume this will fix the rawhide CI failure we have with ocaml > > > > now. > > > > > > Well the previous code didn't compile with OCaml 4.10 at all, so I > > > guess so. However I wasn't previously aware we even had CI for this. > > > Do you have a link? > > > > Rawhide was succeeding until most recent build: > > > > https://ci.centos.org/view/libvirt/job/libvirt-ocaml-build/ > > > > which I presume matches when the VM got updated to ocaml 4.10 > > While it passes 4.10 and Fedora 30/31 with ocaml 4.9, it fails > on CentOS 7 with 4.05 and Debian with 4.02 OCaml < 4.06 didn't have the C macro Bytes_val, but we can use String_val instead with a bit of casting. (The reason we can't just use String_val in the code is because in OCaml >= 4.10 that macro was changed to return a const as strings are now forced to be immutable). I pushed the obvious patch: https://libvirt.org/git/?p=libvirt-ocaml.git;a=commit;h=db1e05d99b3cb195b19d531a8832b980e155cb1f That should fix CentOS 7 (I tested RHEL 7). I'm fairly sure it will also fix Debian 9 (OCaml 4.02), but I don't have that convenient at the moment. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Re: [PATCH ocaml 0/3] Small fixes for OCaml 4.10.
On Fri, Jan 24, 2020 at 04:13:14PM +, Daniel P. Berrangé wrote: > On Fri, Jan 24, 2020 at 03:01:51PM +, Daniel P. Berrangé wrote: > > On Fri, Jan 24, 2020 at 02:51:39PM +, Richard W.M. Jones wrote: > > > On Fri, Jan 24, 2020 at 02:35:01PM +, Daniel P. Berrangé wrote: > > > > On Sun, Jan 19, 2020 at 01:04:07PM +, Richard W.M. Jones wrote: > > > > > Basically OCaml 4.10 is much more strict about the difference between > > > > > string (immutable) and bytes (mutable). > > > > > > > > For all patches > > > > > > > > Reviewed-by: Daniel P. Berrangé > > > > > > > > I presume this will fix the rawhide CI failure we have with ocaml > > > > now. > > > > > > Well the previous code didn't compile with OCaml 4.10 at all, so I > > > guess so. However I wasn't previously aware we even had CI for this. > > > Do you have a link? > > > > Rawhide was succeeding until most recent build: > > > > https://ci.centos.org/view/libvirt/job/libvirt-ocaml-build/ > > > > which I presume matches when the VM got updated to ocaml 4.10 > > While it passes 4.10 and Fedora 30/31 with ocaml 4.9, it fails > on CentOS 7 with 4.05 and Debian with 4.02 We probably need some #define Bytes_val String_val to cope with the older versions of OCaml. I'll have a look when I'm back home. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
Re: [PATCH ocaml 0/3] Small fixes for OCaml 4.10.
Seems like the CI test is passing now. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Re: [PATCH ocaml 0/3] Small fixes for OCaml 4.10.
I pushed those patches, so hopefully you should see the CI job being fixed shortly. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
Re: [PATCH ocaml 0/3] Small fixes for OCaml 4.10.
On Fri, Jan 24, 2020 at 03:01:51PM +, Daniel P. Berrangé wrote: > On Fri, Jan 24, 2020 at 02:51:39PM +0000, Richard W.M. Jones wrote: > > On Fri, Jan 24, 2020 at 02:35:01PM +, Daniel P. Berrangé wrote: > > > On Sun, Jan 19, 2020 at 01:04:07PM +0000, Richard W.M. Jones wrote: > > > > Basically OCaml 4.10 is much more strict about the difference between > > > > string (immutable) and bytes (mutable). > > > > > > For all patches > > > > > > Reviewed-by: Daniel P. Berrangé > > > > > > I presume this will fix the rawhide CI failure we have with ocaml > > > now. > > > > Well the previous code didn't compile with OCaml 4.10 at all, so I > > guess so. However I wasn't previously aware we even had CI for this. > > Do you have a link? > > Rawhide was succeeding until most recent build: > > https://ci.centos.org/view/libvirt/job/libvirt-ocaml-build/ Yes, that's all caused by the OCaml 4.10 update. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Re: [PATCH ocaml 0/3] Small fixes for OCaml 4.10.
On Fri, Jan 24, 2020 at 02:35:01PM +, Daniel P. Berrangé wrote: > On Sun, Jan 19, 2020 at 01:04:07PM +0000, Richard W.M. Jones wrote: > > Basically OCaml 4.10 is much more strict about the difference between > > string (immutable) and bytes (mutable). > > For all patches > > Reviewed-by: Daniel P. Berrangé > > I presume this will fix the rawhide CI failure we have with ocaml > now. Well the previous code didn't compile with OCaml 4.10 at all, so I guess so. However I wasn't previously aware we even had CI for this. Do you have a link? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
[PATCH ocaml 2/3] String_val returns const char * in OCaml 4.10.
This should be compatible with earlier versions of OCaml
too since we are just assigning a char * to a const char *.
---
libvirt/generator.pl| 14 +++---
libvirt/libvirt_c_oneoffs.c | 6 +++---
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/libvirt/generator.pl b/libvirt/generator.pl
index ac3dd65..aff371b 100755
--- a/libvirt/generator.pl
+++ b/libvirt/generator.pl
@@ -593,7 +593,7 @@ sub gen_c_code
} elsif ($sig =~ /^(\w+), string : unit$/) {
"\
" . gen_unpack_args ($1) . "
- char *str = String_val (strv);
+ const char *str = String_val (strv);
int r;
NONBLOCKING (r = $c_name ($1, str));
@@ -605,7 +605,7 @@ sub gen_c_code
"\
CAMLlocal1 (rv);
" . gen_unpack_args ($1) . "
- char *str = String_val (strv);
+ const char *str = String_val (strv);
int r;
NONBLOCKING (r = $c_name ($1, str, 0));
@@ -618,7 +618,7 @@ sub gen_c_code
"\
CAMLlocal1 (rv);
" . gen_unpack_args ($1) . "
- char *str = String_val (strv);
+ const char *str = String_val (strv);
$c_ret_type r;
NONBLOCKING (r = $c_name ($1, str));
@@ -633,7 +633,7 @@ sub gen_c_code
"\
CAMLlocal1 (rv);
" . gen_unpack_args ($1) . "
- char *str = String_val (strv);
+ const char *str = String_val (strv);
$c_ret_type r;
NONBLOCKING (r = $c_name ($1, str, 0));
@@ -648,7 +648,7 @@ sub gen_c_code
"\
CAMLlocal1 (rv);
" . gen_unpack_args ($1) . "
- char *str = String_val (strv);
+ const char *str = String_val (strv);
unsigned int u = Int_val (uv);
$c_ret_type r;
@@ -735,7 +735,7 @@ sub gen_c_code
"\
CAMLlocal2 (rv, connv);
" . gen_unpack_args ($1) . "
- char *str = String_val (strv);
+ const char *str = String_val (strv);
$c_ret_type r;
NONBLOCKING (r = $c_name ($1, str));
@@ -751,7 +751,7 @@ sub gen_c_code
"\
CAMLlocal2 (rv, connv);
" . gen_unpack_args ($1) . "
- char *str = String_val (strv);
+ const char *str = String_val (strv);
$c_ret_type r;
NONBLOCKING (r = $c_name ($1, str, 0));
diff --git a/libvirt/libvirt_c_oneoffs.c b/libvirt/libvirt_c_oneoffs.c
index 8468c73..fc2ac13 100644
--- a/libvirt/libvirt_c_oneoffs.c
+++ b/libvirt/libvirt_c_oneoffs.c
@@ -601,7 +601,7 @@ ocaml_libvirt_domain_set_scheduler_parameters (value domv,
value paramsv)
int nparams = Wosize_val (paramsv);
virSchedParameterPtr params;
int r, i;
- char *name;
+ const char *name;
params = malloc (sizeof (*params) * nparams);
if (params == NULL)
@@ -1005,7 +1005,7 @@ ocaml_libvirt_domain_block_stats (value domv, value pathv)
CAMLparam2 (domv, pathv);
CAMLlocal2 (rv,v);
virDomainPtr dom = Domain_val (domv);
- char *path = String_val (pathv);
+ const char *path = String_val (pathv);
struct _virDomainBlockStats stats;
int r;
@@ -1028,7 +1028,7 @@ ocaml_libvirt_domain_interface_stats (value domv, value
pathv)
CAMLparam2 (domv, pathv);
CAMLlocal2 (rv,v);
virDomainPtr dom = Domain_val (domv);
- char *path = String_val (pathv);
+ const char *path = String_val (pathv);
struct _virDomainInterfaceStats stats;
int r;
--
2.24.1
[PATCH ocaml 3/3] Don't try to memcpy into a String_val.
In OCaml 4.10 String_val returns const char *, so we cannot use it as
the destination for memcpy. Use Bytes_val instead.
---
libvirt/generator.pl| 2 +-
libvirt/libvirt_c_oneoffs.c | 8
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/libvirt/generator.pl b/libvirt/generator.pl
index aff371b..463a19b 100755
--- a/libvirt/generator.pl
+++ b/libvirt/generator.pl
@@ -440,7 +440,7 @@ sub gen_c_code
/* UUIDs are byte arrays with a fixed length. */
rv = caml_alloc_string (VIR_UUID_BUFLEN);
- memcpy (String_val (rv), uuid, VIR_UUID_BUFLEN);
+ memcpy (Bytes_val (rv), uuid, VIR_UUID_BUFLEN);
CAMLreturn (rv);
"
} elsif ($sig =~ /^(\w+) : uuid string$/) {
diff --git a/libvirt/libvirt_c_oneoffs.c b/libvirt/libvirt_c_oneoffs.c
index fc2ac13..e8472b7 100644
--- a/libvirt/libvirt_c_oneoffs.c
+++ b/libvirt/libvirt_c_oneoffs.c
@@ -394,7 +394,7 @@ ocaml_libvirt_connect_call_auth_default_callback (value
listv)
elemv = caml_alloc (2, 0);
if (cred->result != NULL && cred->resultlen > 0) {
v = caml_alloc_string (cred->resultlen);
-memcpy (String_val (v), cred->result, cred->resultlen);
+memcpy (Bytes_val (v), cred->result, cred->resultlen);
optv = caml_alloc (1, 0);
Store_field (optv, 0, v);
} else
@@ -715,7 +715,7 @@ ocaml_libvirt_domain_get_vcpus (value domv, value maxinfov,
value maplenv)
/* Copy the bitmap. */
strv = caml_alloc_string (maxinfo * maplen);
- memcpy (String_val (strv), cpumaps, maxinfo * maplen);
+ memcpy (Bytes_val (strv), cpumaps, maxinfo * maplen);
/* Allocate the tuple and return it. */
rv = caml_alloc_tuple (3);
@@ -900,7 +900,7 @@ ocaml_libvirt_domain_get_all_domain_stats (value connv,
*/
v = caml_alloc_string (VIR_UUID_BUFLEN);
virDomainGetUUID (rstats[i]->dom, uuid);
-memcpy (String_val (v), uuid, VIR_UUID_BUFLEN);
+memcpy (Bytes_val (v), uuid, VIR_UUID_BUFLEN);
Store_field (dsv, 0, v);
tpv = caml_alloc (rstats[i]->nparams, 0); /* typed_param array */
@@ -1646,7 +1646,7 @@ ocaml_libvirt_secret_get_value (value secv)
CHECK_ERROR (secval == NULL, "virSecretGetValue");
rv = caml_alloc_string (size);
- memcpy (String_val (rv), secval, size);
+ memcpy (Bytes_val (rv), secval, size);
free (secval);
CAMLreturn (rv);
--
2.24.1
[PATCH ocaml 0/3] Small fixes for OCaml 4.10.
Basically OCaml 4.10 is much more strict about the difference between string (immutable) and bytes (mutable). Rich.
[PATCH ocaml 1/3] block_peek, memory_peek: Use bytes for return buffer.
Strings are immutable in modern OCaml.
---
libvirt/libvirt.ml | 4 ++--
libvirt/libvirt.mli | 4 ++--
libvirt/libvirt_c_oneoffs.c | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/libvirt/libvirt.ml b/libvirt/libvirt.ml
index 7f9d0e4..bdb9460 100644
--- a/libvirt/libvirt.ml
+++ b/libvirt/libvirt.ml
@@ -731,8 +731,8 @@ struct
external migrate : [>`W] t -> [>`W] Connect.t -> migrate_flag list ->
?dname:string -> ?uri:string -> ?bandwidth:int -> unit -> rw t =
"ocaml_libvirt_domain_migrate_bytecode" "ocaml_libvirt_domain_migrate_native"
external block_stats : [>`R] t -> string -> block_stats =
"ocaml_libvirt_domain_block_stats"
external interface_stats : [>`R] t -> string -> interface_stats =
"ocaml_libvirt_domain_interface_stats"
- external block_peek : [>`W] t -> string -> int64 -> int -> string -> int ->
unit = "ocaml_libvirt_domain_block_peek_bytecode"
"ocaml_libvirt_domain_block_peek_native"
- external memory_peek : [>`W] t -> memory_flag list -> int64 -> int -> string
-> int -> unit = "ocaml_libvirt_domain_memory_peek_bytecode"
"ocaml_libvirt_domain_memory_peek_native"
+ external block_peek : [>`W] t -> string -> int64 -> int -> bytes -> int ->
unit = "ocaml_libvirt_domain_block_peek_bytecode"
"ocaml_libvirt_domain_block_peek_native"
+ external memory_peek : [>`W] t -> memory_flag list -> int64 -> int -> bytes
-> int -> unit = "ocaml_libvirt_domain_memory_peek_bytecode"
"ocaml_libvirt_domain_memory_peek_native"
external get_all_domain_stats : [>`R] Connect.t -> stats_type list ->
get_all_domain_stats_flag list -> domain_stats_record array =
"ocaml_libvirt_domain_get_all_domain_stats"
diff --git a/libvirt/libvirt.mli b/libvirt/libvirt.mli
index 0d74199..7900392 100644
--- a/libvirt/libvirt.mli
+++ b/libvirt/libvirt.mli
@@ -708,7 +708,7 @@ sig
val interface_stats : [>`R] t -> string -> interface_stats
(** Returns network interface stats. *)
- val block_peek : [>`W] t -> string -> int64 -> int -> string -> int -> unit
+ val block_peek : [>`W] t -> string -> int64 -> int -> bytes -> int -> unit
(** [block_peek dom path offset size buf boff] reads [size] bytes at
[offset] in the domain's [path] block device.
@@ -717,7 +717,7 @@ sig
See also {!max_peek}. *)
val memory_peek : [>`W] t -> memory_flag list -> int64 -> int ->
-string -> int -> unit
+bytes -> int -> unit
(** [memory_peek dom Virtual offset size] reads [size] bytes
at [offset] in the domain's virtual memory.
diff --git a/libvirt/libvirt_c_oneoffs.c b/libvirt/libvirt_c_oneoffs.c
index 40384e8..8468c73 100644
--- a/libvirt/libvirt_c_oneoffs.c
+++ b/libvirt/libvirt_c_oneoffs.c
@@ -1057,7 +1057,7 @@ ocaml_libvirt_domain_block_peek_native (value domv, value
pathv, value offsetv,
const char *path = String_val (pathv);
unsigned long long offset = Int64_val (offsetv);
size_t size = Int_val (sizev);
- char *buffer = String_val (bufferv);
+ unsigned char *buffer = Bytes_val (bufferv);
int boff = Int_val (boffv);
int r;
@@ -1089,7 +1089,7 @@ ocaml_libvirt_domain_memory_peek_native (value domv,
value flagsv, value offsetv
int flags = 0;
unsigned long long offset = Int64_val (offsetv);
size_t size = Int_val (sizev);
- char *buffer = String_val (bufferv);
+ unsigned char *buffer = Bytes_val (bufferv);
int boff = Int_val (boffv);
int r;
--
2.24.1
Re: [libvirt PATCH] qemu: fixing auto-detecting binary in domain capabilities
On Fri, Jan 17, 2020 at 01:29:36PM +, Daniel P. Berrangé wrote:
> The virConnectGetDomainCapabilities API accepts either a binary path
> to the emulator, or desired guest arch. If guest arch is not given,
> then the host arch is assumed.
>
> In the case where the binary is not given, the code tried to find the
> emulator binary in the existing list of cached emulator capabilities.
> This is not valid since we switched to lazy population of the cache in:
>
> commit 3dd91af01f30c5bda6328454ef49f3afece755d6
> Author: Daniel P. Berrangé
> Date: Mon Dec 2 13:04:26 2019 +
>
> qemu: stop creating capabilities at driver startup
>
> As a result of this change, if there are no persistent guests defined
> using the requested guest architecture, virConnectGetDomainCapabilities
> will fail to find an emulator binary.
>
> The solution is to stop relying on the cached capabilities to find the
> binary and instead use the same logic we use to pick default a binary
> per arch when populating capabilities.
>
> Signed-off-by: Daniel P. Berrangé
Fixes the problem over here, so:
Tested-by: Richard W.M. Jones
Thanks,
Rich.
> ---
> src/qemu/qemu_capabilities.c | 42
> 1 file changed, 19 insertions(+), 23 deletions(-)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 498348ad58..9017e8d920 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -5280,10 +5280,12 @@ virQEMUCapsCacheLookupDefault(virFileCachePtr cache,
>const char **retMachine)
> {
> int virttype = VIR_DOMAIN_VIRT_NONE;
> -int arch = virArchFromHost();
> +virArch hostarch = virArchFromHost();
> +virArch arch = hostarch;
> virDomainVirtType capsType;
> virQEMUCapsPtr qemuCaps = NULL;
> virQEMUCapsPtr ret = NULL;
> +virArch arch_from_caps;
>
> if (virttypeStr &&
> (virttype = virDomainVirtTypeFromString(virttypeStr)) < 0) {
> @@ -5299,31 +5301,25 @@ virQEMUCapsCacheLookupDefault(virFileCachePtr cache,
> goto cleanup;
> }
>
> -if (binary) {
> -virArch arch_from_caps;
> +if (!binary)
> +binary = virQEMUCapsGetDefaultEmulator(hostarch, arch);
>
> -if (!(qemuCaps = virQEMUCapsCacheLookup(cache, binary)))
> -goto cleanup;
> +if (!(qemuCaps = virQEMUCapsCacheLookup(cache, binary)))
> +goto cleanup;
>
> -arch_from_caps = virQEMUCapsGetArch(qemuCaps);
> +arch_from_caps = virQEMUCapsGetArch(qemuCaps);
>
> -if (arch_from_caps != arch &&
> -!((ARCH_IS_X86(arch) && ARCH_IS_X86(arch_from_caps)) ||
> - (ARCH_IS_PPC(arch) && ARCH_IS_PPC(arch_from_caps)) ||
> - (ARCH_IS_ARM(arch) && ARCH_IS_ARM(arch_from_caps)) ||
> - (ARCH_IS_S390(arch) && ARCH_IS_S390(arch_from_caps {
> -virReportError(VIR_ERR_INVALID_ARG,
> - _("architecture from emulator '%s' doesn't "
> - "match given architecture '%s'"),
> - virArchToString(arch_from_caps),
> - virArchToString(arch));
> -goto cleanup;
> -}
> -} else {
> -if (!(qemuCaps = virQEMUCapsCacheLookupByArch(cache, arch)))
> -goto cleanup;
> -
> -binary = virQEMUCapsGetBinary(qemuCaps);
> +if (arch_from_caps != arch &&
> +!((ARCH_IS_X86(arch) && ARCH_IS_X86(arch_from_caps)) ||
> + (ARCH_IS_PPC(arch) && ARCH_IS_PPC(arch_from_caps)) ||
> + (ARCH_IS_ARM(arch) && ARCH_IS_ARM(arch_from_caps)) ||
> + (ARCH_IS_S390(arch) && ARCH_IS_S390(arch_from_caps {
> +virReportError(VIR_ERR_INVALID_ARG,
> + _("architecture from emulator '%s' doesn't "
> + "match given architecture '%s'"),
> + virArchToString(arch_from_caps),
> + virArchToString(arch));
> +goto cleanup;
> }
>
> if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
> --
> 2.23.0
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
Re: [libvirt RFC 0/2] qemu: handle 'raw' format options 'size' and 'offset'
On Thu, Jan 16, 2020 at 05:17:43PM +, Richard W.M. Jones wrote: > On Thu, Jan 16, 2020 at 05:56:38PM +0100, Peter Krempa wrote: > > See patch 2 please. > > > > Peter Krempa (2): > > tests: storage: Add test data for json specified raw image with > > offset/size > > qemu: Handle 'offset' and 'size' parameters of 'raw' driver > > > > src/conf/domain_conf.c| 19 +++ > > src/qemu/qemu_block.c | 10 -- > > src/util/virstoragefile.c | 21 + > > src/util/virstoragefile.h | 4 > > tests/virstoragetest.c| 15 +++ > > 5 files changed, 59 insertions(+), 10 deletions(-) > > Can confirm that this fixes the issue in the virt-v2v test suite. Actually I'm sorry to say it didn't fix it. I'm not sure what exactly happened there. I might have missed the error message or I ran the test suite in the wrong way, or perhaps the error is intermittent. Anyway I have to withdraw the "Tested-by" until I can take a closer look at this. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Re: [libvirt RFC 0/2] qemu: handle 'raw' format options 'size' and 'offset'
On Thu, Jan 16, 2020 at 05:56:38PM +0100, Peter Krempa wrote: > See patch 2 please. > > Peter Krempa (2): > tests: storage: Add test data for json specified raw image with > offset/size > qemu: Handle 'offset' and 'size' parameters of 'raw' driver > > src/conf/domain_conf.c| 19 +++ > src/qemu/qemu_block.c | 10 -- > src/util/virstoragefile.c | 21 + > src/util/virstoragefile.h | 4 > tests/virstoragetest.c| 15 +++ > 5 files changed, 59 insertions(+), 10 deletions(-) Can confirm that this fixes the issue in the virt-v2v test suite. Tested-by: Richard W.M. Jones Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
[PATCH 2/2] run.in: Include tools directory on $PATH.
You normally want to run the locally compiled copy of virsh. Trying to run the installed version with the locally compiled library is a recipe for problems with missing symbols and so on. By adding tools to the path we can ensure that (eg) the libguestfs test suite will use compatible copies of the library and virsh. Signed-off-by: Richard W.M. Jones --- run.in | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/run.in b/run.in index 3118f9a9a4..d508e64802 100644 --- a/run.in +++ b/run.in @@ -21,12 +21,10 @@ # With this script you can run libvirt programs without needing to # install them first. You just have to do for example: # -# ./run ./tools/virsh [args ...] +# ./run virsh [args ...] # -# If you are already in the tools/ subdirectory, then the following -# command will also work: -# -# ../run ./virsh [...] +# Note that this runs the locally compiled copy of virsh which +# is usually want you want. # # You can also run the C programs under valgrind like this: # @@ -38,7 +36,7 @@ # # This also works with sudo (eg. if you need root access for libvirt): # -# sudo ./run ./tools/virsh list --all +# sudo ./run virsh list --all # #-- @@ -58,6 +56,9 @@ export LD_LIBRARY_PATH prepend PKG_CONFIG_PATH "$b/src" export PKG_CONFIG_PATH +prepend PATH "$b/tools" +export PATH + # Ensure that any 3rd party apps using libvirt.so from the build tree get # files resolved to the build/source tree too. Typically useful for language # bindings running tests against non-installed libvirt. -- 2.24.1
[PATCH 1/2] run.in: Add intelligent prepend function.
This has been used in libguestfs and libnbd for quite a while as it
makes the ./run script easier to read and write.
See also:
http://stackoverflow.com/a/9631350
Signed-off-by: Richard W.M. Jones
---
run.in | 20 +---
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/run.in b/run.in
index 8574f81463..3118f9a9a4 100644
--- a/run.in
+++ b/run.in
@@ -42,22 +42,20 @@
#
#--
+# Function to intelligently prepend a path to an environment variable.
+# See http://stackoverflow.com/a/9631350
+prepend()
+{
+eval $1="$2\${$1:+:\$$1}"
+}
+
# Find this script.
b=@abs_builddir@
-library_path="$b/src/.libs"
-if [ -z "$LD_LIBRARY_PATH" ]; then
-LD_LIBRARY_PATH=$library_path
-else
-LD_LIBRARY_PATH="$library_path:$LD_LIBRARY_PATH"
-fi
+prepend LD_LIBRARY_PATH "$b/src/.libs"
export LD_LIBRARY_PATH
-if [ -z "$PKG_CONFIG_PATH" ]; then
-PKG_CONFIG_PATH="$b/src"
-else
-PKG_CONFIG_PATH="$b/src:$PKG_CONFIG_PATH"
-fi
+prepend PKG_CONFIG_PATH "$b/src"
export PKG_CONFIG_PATH
# Ensure that any 3rd party apps using libvirt.so from the build tree get
--
2.24.1
[PATCH 0/2] Simple fixes for run.in
The ./run script is a very useful way to run the libguestfs and virt-v2v test suites against a locally compiled copy of libvirt. These two patches contain some minor fixes. Rich.
Re: [libvirt] [PATCH] util: storagefile: Properly set transport type when parsing NBD strings
On Thu, Jan 16, 2020 at 01:13:38PM +, Richard W.M. Jones wrote: > On Thu, Jan 16, 2020 at 12:41:37PM +0100, Peter Krempa wrote: > > When parsing legacy NBD backing file strings such as > > 'nbd:unix:/tmp/sock:exportname=/' we'd fail to set the transport to > > VIR_STORAGE_NET_HOST_TRANS_UNIX. This started to be a problem once we > > actually started to generate config of the backing store on the command > > line with -blockdev as the JSON code would try to format it as TCP and > > fail with: > > > > internal error: argument key 'host' must not have null value > > > > Set the type properly and add a test. > > > > This bug was found by the libguestfs test suite in: > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1791614 > > The bug was found by Ming Xie of the virt-v2v QE team. > > The patch itself looks fine as far as I can tell. But I'm having real > problems actually testing it. Can you suggest any way to test libvirt > parsing these URIs? > > My current method (which doesn't work for reasons that I don't > understand) is: > > (1) Compile libvirt from source. > > (2) Run ./run src/libvirtd & > > (3) Create an NBD server + overlay file: > > rm /tmp/sock > nbdkit -U /tmp/sock memory 1G > qemu-img create -f qcow2 overlay.qcow2 -b nbd:unix:/tmp/sock:exportname=/ -F > raw > > (4) Try to boot a libvirt guest using the overlay: > > virt-install --import --name test --disk path=overlay.qcow2,format=raw > --memory 1024 > > But for some reason libvirt just ignores the overlay: > > 2020-01-16 13:00:44.980+: 2378711: warning : > virStorageBackendVolOpen:1527 : ignoring missing file 'nbd:unix:/tmp/sock' > > I verified that the overlay works in programs like qemu-img so it > doesn't seem to be a problem with the overlay itself. The problem was too old qemu. With qemu 4.2 I can verify that the patch is working, so: Tested-by: Richard W.M. Jones Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Re: [libvirt] [PATCH] util: storagefile: Properly set transport type when parsing NBD strings
On Thu, Jan 16, 2020 at 12:41:37PM +0100, Peter Krempa wrote:
> When parsing legacy NBD backing file strings such as
> 'nbd:unix:/tmp/sock:exportname=/' we'd fail to set the transport to
> VIR_STORAGE_NET_HOST_TRANS_UNIX. This started to be a problem once we
> actually started to generate config of the backing store on the command
> line with -blockdev as the JSON code would try to format it as TCP and
> fail with:
>
> internal error: argument key 'host' must not have null value
>
> Set the type properly and add a test.
>
> This bug was found by the libguestfs test suite in:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1791614
The bug was found by Ming Xie of the virt-v2v QE team.
The patch itself looks fine as far as I can tell. But I'm having real
problems actually testing it. Can you suggest any way to test libvirt
parsing these URIs?
My current method (which doesn't work for reasons that I don't
understand) is:
(1) Compile libvirt from source.
(2) Run ./run src/libvirtd &
(3) Create an NBD server + overlay file:
rm /tmp/sock
nbdkit -U /tmp/sock memory 1G
qemu-img create -f qcow2 overlay.qcow2 -b nbd:unix:/tmp/sock:exportname=/ -F raw
(4) Try to boot a libvirt guest using the overlay:
virt-install --import --name test --disk path=overlay.qcow2,format=raw --memory
1024
But for some reason libvirt just ignores the overlay:
2020-01-16 13:00:44.980+: 2378711: warning : virStorageBackendVolOpen:1527
: ignoring missing file 'nbd:unix:/tmp/sock'
I verified that the overlay works in programs like qemu-img so it
doesn't seem to be a problem with the overlay itself.
Rich.
> Signed-off-by: Peter Krempa
> ---
> src/util/virstoragefile.c | 2 +-
> tests/virstoragetest.c| 4
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index 1397f532fd..7a2af0ad94 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -2964,7 +2964,7 @@ virStorageSourceParseNBDColonString(const char *nbdstr,
> }
>
> src->hosts->socket = g_strdup(backing[2]);
> -
> +src->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_UNIX;
> } else {
> src->hosts->name = g_strdup(backing[1]);
>
> diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
> index 2862758752..370e19252b 100644
> --- a/tests/virstoragetest.c
> +++ b/tests/virstoragetest.c
> @@ -1258,6 +1258,10 @@ mymain(void)
> "\n"
> " \n"
> "\n");
> +TEST_BACKING_PARSE("nbd:unix:/tmp/sock:exportname=/",
> + "\n"
> + " \n"
> + "\n");
> TEST_BACKING_PARSE("nbd://example.org:1234",
> "\n"
> " \n"
> --
> 2.24.1
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: stop clearing QEMU emulator capabilities
On Thu, Nov 28, 2019 at 01:04:00PM +, Daniel P. Berrangé wrote: > We have an RFE from libguestfs to provide a way to run as root > *with* capabilities. I looked integrating this into the DAC security > manager as a new flag in the security label, but then I started > thinking about the whole idea of clearing capabilities > > https://bugzilla.redhat.com/show_bug.cgi?id=1045039 > > Pretty much forever we have explicitly cleared QEMU emulator > capabilities when starting it. > > When QEMU uid/gid is set to non-root this is pointless as if we just > used a regular setuid/setgid call, the process will have all its > capabilities cleared anyway by the kernel. > > When QEMU uid/gid is set to root, this is almost (always?) never > what people actually want. People make QEMU run as root in order > to access some privileged resource that libvirt doesn't support > yet and this often requires capabilities. As a result they have > to go find the qemu.conf param to turn this off. This is not > viable for libguestfs - they want to control everything via thue > XML security label to request running as root regardless of the > qemu.conf settings for user/group. > > Clearing capabilities was implemented originally because there > was a proposal in Fedora to change permissions such that root, > with no capabilities would not be able to compromise the system. > ie a locked down root account. This never went anywhere though, > and as a result clearing capabilities when running as root does > not really get us any security benefit AFAICT. The root user > can just do something like create a cronjob, which will then > faithfully be run with full capabilities, trivially bypassing > the restriction we place. > > IOW, our clearing of capabilities is both useless from a security > POV, and breaks valid use cases when people need to run as root. > > I'm thinking we should just rip out the code which clears capabilities > and allow default loggic to run > > - If uid/gid is non-root, then no capabilities are present > - If uid/gid is root, then full capabilities are present All seems reasonable to me ... Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/75] Drop virAsprintf() in favor of g_strdup_printf()
On Wed, Oct 23, 2019 at 10:18:58AM +0200, Michal Privoznik wrote: [...] Thanks for the explanation. In case you didn't discover it, cocci has a very responsive upstream mailing list: https://systeme.lip6.fr/mailman/listinfo/cocci > 1: Thing is, while coccinelle understands C very well, it doesn't quite > understand all those __attribute__ and other extensions. Therefore, > some macros > made it impossible for coccinelle to parse some functions (e.g. g_autofree, > g_autoptr and similar) and it did not perform requested change there then. They might be interested in if there's anything that can be done about this. In libguestfs, nbdkit etc we are extensively using the cleanup attribute, and so is systemd. I've even seen [somewhere, can't find it at the moment] a proposal to add the equivalent feature to the next C standard. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] security_stack: Perform rollback if one of stacked drivers fails
On Wed, Oct 09, 2019 at 07:49:29PM -0400, Cole Robinson wrote: > In that bug, I see that rjones (cc'd) said that libvirt not > remembering labels/uid causes issues for libguestfs that requires > workarounds. Rich, do you have links to threads or bug reports where > this is described in more detail? I think there are two problems (which I often confuse) and they are possibly related. This one where libvirt doesn't restore permissions afterwards, and the other one where qemu:///session cannot be used as root which implies that when you run libguestfs as root it doesn't have access to things that root would normally have access to (bug 890291 / 1045069). In answer to your question this is the only one I could find which is definitely related to this bug: https://www.redhat.com/archives/libguestfs/2013-May/msg00115.html Here's another one, but I think this is related to the other bug: https://bugs.launchpad.net/nova/+bug/1241659/comments/6 I suspect there are cases where openstack sets LIBGUESTFS_BACKEND=direct to workaround one of these two bugs. Is fixing the qemu:///session as root problem going to also solve this? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ocaml PATCH] Use caml_raise_out_of_memory() where needed.
On Thu, Sep 05, 2019 at 02:54:53PM +0200, Pino Toscano wrote:
> Raise the proper exception on malloc failures.
>
> Signed-off-by: Pino Toscano
> ---
> libvirt/libvirt_c_oneoffs.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/libvirt/libvirt_c_oneoffs.c b/libvirt/libvirt_c_oneoffs.c
> index e23c0db..40384e8 100644
> --- a/libvirt/libvirt_c_oneoffs.c
> +++ b/libvirt/libvirt_c_oneoffs.c
> @@ -749,7 +749,7 @@ ocaml_libvirt_domain_get_cpu_stats (value domv)
>CHECK_ERROR (nparams < 0, "virDomainGetCPUStats");
>
>if ((params = malloc(sizeof(*params) * nparams * 128)) == NULL)
> -caml_failwith ("virDomainGetCPUStats: malloc");
> +caml_raise_out_of_memory ();
>
>cpustats = caml_alloc (nr_pcpus, 0); /* cpustats: array of params(list of
> typed_param) */
>cpu = 0;
> @@ -1461,7 +1461,7 @@ ocaml_libvirt_event_add_timeout (value connv, value ms,
> value callback_id)
>/* Store the int64 callback_id as the opaque data so the OCaml
> callback can demultiplex to the correct OCaml handler. */
>if ((opaque = malloc(sizeof(long))) == NULL)
> -caml_failwith ("virEventAddTimeout: malloc");
> +caml_raise_out_of_memory ();
>*((long*)opaque) = Int64_val(callback_id);
>NONBLOCKING(r = virEventAddTimeout(Int_val(ms), cb, opaque, freecb));
>CHECK_ERROR(r == -1, "virEventAddTimeout");
> @@ -1551,7 +1551,7 @@ ocaml_libvirt_connect_domain_event_register_any(value
> connv, value domv, value c
>/* Store the int64 callback_id as the opaque data so the OCaml
> callback can demultiplex to the correct OCaml handler. */
>if ((opaque = malloc(sizeof(long))) == NULL)
> -caml_failwith ("virConnectDomainEventRegisterAny: malloc");
> +caml_raise_out_of_memory ();
>*((long*)opaque) = Int64_val(callback_id);
>NONBLOCKING(r = virConnectDomainEventRegisterAny(conn, dom, eventID, cb,
> opaque, freecb));
>CHECK_ERROR(r == -1, "virConnectDomainEventRegisterAny");
ACK.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ocaml PATCH] Make const the return value of caml_named_value()
On Thu, Sep 05, 2019 at 09:59:48AM +0200, Pino Toscano wrote:
> With OCaml >= 4.09 caml_named_value() returns a const value *, so keep
> the constness to build also in this case.
>
> Signed-off-by: Pino Toscano
> ---
> libvirt/libvirt_c_oneoffs.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libvirt/libvirt_c_oneoffs.c b/libvirt/libvirt_c_oneoffs.c
> index 6f56f10..e23c0db 100644
> --- a/libvirt/libvirt_c_oneoffs.c
> +++ b/libvirt/libvirt_c_oneoffs.c
> @@ -1207,7 +1207,7 @@ ocaml_libvirt_event_run_default_impl (value unitv)
> #define DOMAIN_CALLBACK_BEGIN(NAME) \
>value connv, domv, callback_id, result;\
>connv = domv = callback_id = result = Val_int(0); \
> - static value *callback = NULL; \
> + static const value *callback = NULL; \
>caml_leave_blocking_section(); \
>if (callback == NULL) \
> callback = caml_named_value(NAME); \
> @@ -1433,7 +1433,7 @@ timeout_callback(int timer, void *opaque)
> {
>value callback_id, result;
>callback_id = result = Val_int(0);
> - static value *callback = NULL;
> + static const value *callback = NULL;
>caml_leave_blocking_section();
>if (callback == NULL)
> callback = caml_named_value("Libvirt.timeout_callback");
ACK
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ocaml PATCH 7/6] Hide again the internal helpers
On Tue, Sep 03, 2019 at 01:41:16PM +0200, Pino Toscano wrote: > Use a GCC pragma to hide all the internal helpers, so they are not > exposed as public symbols of the stub library. > > Signed-off-by: Pino Toscano > --- > libvirt/libvirt_c.h | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/libvirt/libvirt_c.h b/libvirt/libvirt_c.h > index 541d8e3..45937f6 100644 > --- a/libvirt/libvirt_c.h > +++ b/libvirt/libvirt_c.h > @@ -40,6 +40,13 @@ > > /* Please read libvirt/README file. */ > > +/* Make sure to not expose our internal helpers as public symbols. > + * https://gcc.gnu.org/wiki/Visibility > + */ > +#ifdef __GNUC__ > +#pragma GCC visibility push(hidden) > +#endif > + > const char *Optstring_val (value strv); > typedef value (*Val_ptr_t) (void *); > value Val_opt (void *ptr, Val_ptr_t Val_ptr); > @@ -153,4 +160,8 @@ value Val_pool (virStoragePoolPtr pol, value connv); > value Val_volume (virStorageVolPtr vol, value connv); > value Val_secret (virSecretPtr sec, value connv); > > +#ifdef __GNUC__ > +#pragma GCC visibility pop > +#endif > + > #endif Even better :-) ACK series. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ocaml PATCH 6/6] Rename libvirt_c_epilogue.c to libvirt_c_common.c
This is all fine. Should we try to use a linker script to hide the public symbols from patch 2? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ocaml PATCH 4/4] build: remove the list_secrets binary on clean
On Thu, Aug 29, 2019 at 05:38:05PM +0200, Pino Toscano wrote: > On Thursday, 29 August 2019 16:52:48 CEST Richard W.M. Jones wrote: > > When I was trying to fix this package and do a release last week I was > > thinking how nice it would be if it used automake (or perhaps meson) > > and we could drop the manifest etc. Hint hint! > > Actually, I'm currently playing with dune, and I have to say it feels > nice. There only problematic thing is the current generated > libvirt_c.c that includes the other C files, it does not play well with > the automatic dependencies that dune figures out. > > BTW what is MANIFEST for? IIRC it is used in Perl modules, but not so > much in OCaml ones? It's used by the home-brew ‘make dist’ rule: https://libvirt.org/git/?p=libvirt-ocaml.git;a=blob;f=Makefile.in;h=e50600a22c8a63d39438d85490b94e4a97180392;hb=HEAD#l77 AIUI with automake there would be a properly generated make dist so the whole thing could go away. Rich. > > Are you able to push this to the upstream repo or do you need me to do > > anything there? > > Yup, I am -- pushed already. > > Thanks, > -- > Pino Toscano -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ocaml PATCH 4/4] build: remove the list_secrets binary on clean
This patch series is fine, ACK. When I was trying to fix this package and do a release last week I was thinking how nice it would be if it used automake (or perhaps meson) and we could drop the manifest etc. Hint hint! Are you able to push this to the upstream repo or do you need me to do anything there? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ocaml PATCH 3/3] Fix shebang in perl scripts
On Tue, Aug 20, 2019 at 03:44:56PM +0200, Pino Toscano wrote: > Instead of hardcoding the location of perl (assuming it is installed in > /usr), use /usr/bin/env to run it, and thus picking it from $PATH. > This makes it possible to run these scripts also on installations with > perl in a different prefix than /usr. > > Also, given that we want enable warnings on scripts, turn the -w > previously in shebang to explicit "use warnings;" in scripts which > didn't have it before. > > Signed-off-by: Pino Toscano > --- > libvirt/generator.pl | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libvirt/generator.pl b/libvirt/generator.pl > index 490ef9a..e850500 100755 > --- a/libvirt/generator.pl > +++ b/libvirt/generator.pl > @@ -1,4 +1,4 @@ > -#!/usr/bin/perl -w > +#!/usr/bin/env perl > # > # OCaml bindings for libvirt. > # (C) Copyright 2007-2015 Richard W.M. Jones, Red Hat Inc. > @@ -26,6 +26,7 @@ > # Please read libvirt/README. > > use strict; > +use warnings; All seems very straightforward, ACK series. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ocaml PATCH] build: build with CAML_NAME_SPACE
On Tue, May 28, 2019 at 10:29:43AM +0200, Pino Toscano wrote: > This way no non-namespaced OCaml C symbols are used, reducing the risk > of clashes with other code. > > Signed-off-by: Pino Toscano > --- > libvirt/Makefile.in | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/libvirt/Makefile.in b/libvirt/Makefile.in > index 3a68aed..77462c7 100644 > --- a/libvirt/Makefile.in > +++ b/libvirt/Makefile.in > @@ -19,6 +19,7 @@ WIN32 = @WIN32@ > > CFLAGS = @CFLAGS@ \ > @LIBVIRT_CFLAGS@ \ > +-DCAML_NAME_SPACE \ > -I.. \ > -I"$(shell ocamlc -where)" \ > @DEBUG@ @WARNINGS@ @CFLAGS_FPIC@ ACK. We should probably add that in a few other places too. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ocaml PATCH 0/3] Few misc improvements
On Fri, Apr 05, 2019 at 06:33:52PM +0200, Pino Toscano wrote: > Raise the libvirt version to the de-facto requirement, and add a new > API which will be useful. Also, sync Virterror with libvirt. > > Pino Toscano (3): > build: bump required libvirt to 1.2.8 > Implement Connect.get_domain_capabilities > Synchronize Virterror with libvirt 5.2.0 > > configure.ac | 2 +- > libvirt/libvirt.ml | 12 > libvirt/libvirt.mli | 23 ++- > libvirt/libvirt_c_epilogue.c | 4 ++-- > libvirt/libvirt_c_oneoffs.c | 16 > 5 files changed, 53 insertions(+), 4 deletions(-) Thanks, ACKed and pushed (finally!) Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu-nbd: Deprecate qemu-nbd --partition
On Fri, Jan 25, 2019 at 05:48:37PM -0600, Eric Blake wrote:
> The existing qemu-nbd --partition code claims to handle logical
> partitions up to 8, since its introduction in 2008 (commit 7a5ca86).
> However, the implementation is bogus (actual MBR logical partitions
> form a sort of linked list, with one partition per extended table
> entry, rather than four logical partitions in a single extended
> table), making the code unlikely to work for anything beyond -P5 on
> actual guest images. What's more, the code does not support GPT
> partitions, which are becoming more popular, and maintaining device
> subsetting in both NBD and the raw device is unnecessary duplication
> of effort (even if it is not too difficult).
>
> Note that obtaining the offsets of a partition (MBR or GPT) can be
> learned by using 'qemu-nbd -c /dev/nbd0 file.qcow2 && sfdisk --dump
> /dev/nbd0', but by the time you've done that, you might as well
> just mount /dev/nbd0p1 that the kernel creates for you instead of
> bothering with qemu exporting a subset. Or, keeping to just
> user-space code, use nbdkit's partition filter, which has already
> known both GPT and primary MBR partitions for a while, and was
> just recently enhanced to support arbitrary logical MBR parititions.
>
> Start the clock on the deprecation cycle, with examples of how
> to write device subsetting without using -P.
>
> Signed-off-by: Eric Blake
>
> ---
> v2: actual nbdkit example [Rich], improved doc wording
> ---
> qemu-deprecated.texi | 33 +
> qemu-nbd.texi| 6 --
> qemu-nbd.c | 2 ++
> 3 files changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index 219206a836f..d35e78c81ff 100644
> --- a/qemu-deprecated.texi
> +++ b/qemu-deprecated.texi
> @@ -175,3 +175,36 @@ The above, converted to the current supported format:
> @subsubsection "irq": "" (since 3.0.0)
>
> The ``irq'' property is obsoleted.
> +
> +@section Related binaries
> +
> +@subsection qemu-nbd --partition (since 4.0.0)
> +
> +The ``qemu-nbd --partition $digit'' code (also spelled @option{-P})
> +can only handle MBR partitions, and has never correctly handled
> +logical partitions beyond partition 5. If you know the offset and
> +length of the partition (perhaps by using @code{sfdisk} within the
> +guest), you can achieve the effect of exporting just that subset of
> +the disk by use of the @option{--image-opts} option with a raw
> +blockdev using the @code{offset} and @code{size} parameters layered on
> +top of any other existing blockdev. For example, if partition 1 is
> +100MiB long starting at 1MiB, the old command:
> +
> +@code{qemu-nbd -t -P 1 -f qcow2 file.qcow2}
> +
> +can be rewritten as:
> +
> +@code{qemu-nbd -t --image-opts
> driver=raw,offset=1M,size=100M,file.driver=qcow2,file.backing.driver=file,file.backing.filename=file.qcow2}
> +
> +Alternatively, the @code{nbdkit} project provides a more powerful
> +partition filter on top of its nbd plugin, which can be used to select
> +an arbitrary MBR or GPT partition on top of any other full-image NBD
> +export. Using this to rewrite the above example results in:
> +
> +@code{qemu-nbd -t -k /tmp/sock -f qcow2 file.qcow2 &}
> +@code{nbdkit -f --filter=partition nbd socket=/tmp/sock partition=1}
> +
> +Note that if you are exposing the export via /dev/nbd0, it is easier
> +to just export the entire image and then mount only /dev/nbd0p1 than
> +it is to reinvoke @command{qemu-nbd -c /dev/nbd0} limited to just a
> +subset of the image.
> diff --git a/qemu-nbd.texi b/qemu-nbd.texi
> index 386bece4680..d0c51828149 100644
> --- a/qemu-nbd.texi
> +++ b/qemu-nbd.texi
> @@ -56,8 +56,10 @@ auto-detecting.
> @item -r, --read-only
> Export the disk as read-only.
> @item -P, --partition=@var{num}
> -Only expose MBR partition @var{num}. Understands physical partitions
> -1-4 and logical partitions 5-8.
> +Deprecated: Only expose MBR partition @var{num}. Understands physical
> +partitions 1-4 and logical partition 5. New code should instead use
> +@option{--image-opts} with the raw driver wrapping a subset of the
> +original image.
> @item -B, --bitmap=@var{name}
> If @var{filename} has a qcow2 persistent bitmap @var{name}, expose
> that bitmap via the ``qemu:dirty-bitmap:@var{name}'' context
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 1f7b2a03f5d..00c07fd27ea 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -787,6 +787,8 @@ int main(int argc, char **argv)
> flags &= ~BDRV_O_RDWR;
> break;
> case 'P':
> +warn_report("The '-P' option is depre
Re: [libvirt] [PATCH] qemu-nbd: Deprecate qemu-nbd --partition
On Wed, Jan 23, 2019 at 04:18:16PM -0600, Eric Blake wrote:
> On 1/23/19 3:55 PM, Richard W.M. Jones wrote:
> > On Wed, Jan 23, 2019 at 03:19:53PM -0600, Eric Blake wrote:
> >> The existing qemu-nbd --partition code claims to handle logical
> >> partitions up to 8, since its introduction in 2008 (commit 7a5ca86).
> >> However, the implementation is bogus (actual MBR logical partitions
> >> form a sort of linked list, with one partition per extended table
> >> entry, rather than four logical partitions in a single extended
> >> table), making the code unlikely to work for anything beyond -P5 on
> >> actual guest images. What's more, the code does not support GPT
> >> partitions, which are becoming more popular, and maintaining device
> >> subsetting in both NBD and the raw device is unnecessary maintenance
> >> burden. And nbdkit has just added code to properly handle an
> >> arbitrary number of MBR partitions, along with its existing code
> >> for handling GPT partitions.
> >>
> >> Note that obtaining the offsets of a partition can be learned by
> >> using 'qemu-nbd -c /dev/nbd0 file.qcow2 && sfdisk --dump /dev/nbd0',
> >> but by the time you've done that, you might as well just mount
> >> /dev/nbd0p1 that the kernel creates for you.
> >>
> >> Start the clock on the deprecation cycle, with an example of how
> >> to write device subsetting without using -P.
> >>
>
> >> +For example, if partition 1 is 100MiB starting at 1MiB, the old command
> >> +
> >> +@example{qemu-nbd -P 1 -f qcow2 file.qcow2}
> >> +
> >> +can be rewritten as:
> >> +
> >> +@example{qemu-nbd --image-opts
> >> driver=raw,offset=1M,size=100M,file.driver=qcow2,file.backing.driver=file,file.backing.filename=file.qcow2}
> >> +
> >> +Alternatively, the @code{nbdkit} project provides a more powerful
> >> +partition filter on top of its nbd plugin, which can be used to select
> >> +an arbitrary MBR or GPT partition on top of any other full-image NBD
> >> +export.
> >
> > You might want to add the actual command here.
>
> Good idea - as long as we are deprecating something, telling the user
> how to get the same functionality (in this case, user-space partition
> detection, without involving /dev/nbd) is worth the extra effort.
>
> > Unfortunately nbdkit
> > cannot read qcow2 files meaning (as you note already) that you have to
> > forward the connection through the nbdkit-nbd-plugin to qemu-nbd.
> > This worked for me:
> >
> > qemu-nbd -t -k /tmp/sock -f qcow2 file.qcow2 &
> > nbdkit -f --filter=partition nbd socket=/tmp/sock partition=1 &
>
> Is the -f necessary? Otherwise, yes, this looks reasonable. I'll add it
> for v2.
It's not necessary, but it makes nbdkit behave the same way with
respect to remaining in the foreground as qemu-nbd.
Rich.
> > If you drop the requirement to demonstrate this with qcow2 then the
> > command would be just this:
> >
> > nbdkit --filter=partition file disk.raw partition=1
> >
>
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3226
> Virtualization: qemu.org | libvirt.org
>
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu-nbd: Deprecate qemu-nbd --partition
On Wed, Jan 23, 2019 at 03:19:53PM -0600, Eric Blake wrote:
> The existing qemu-nbd --partition code claims to handle logical
> partitions up to 8, since its introduction in 2008 (commit 7a5ca86).
> However, the implementation is bogus (actual MBR logical partitions
> form a sort of linked list, with one partition per extended table
> entry, rather than four logical partitions in a single extended
> table), making the code unlikely to work for anything beyond -P5 on
> actual guest images. What's more, the code does not support GPT
> partitions, which are becoming more popular, and maintaining device
> subsetting in both NBD and the raw device is unnecessary maintenance
> burden. And nbdkit has just added code to properly handle an
> arbitrary number of MBR partitions, along with its existing code
> for handling GPT partitions.
>
> Note that obtaining the offsets of a partition can be learned by
> using 'qemu-nbd -c /dev/nbd0 file.qcow2 && sfdisk --dump /dev/nbd0',
> but by the time you've done that, you might as well just mount
> /dev/nbd0p1 that the kernel creates for you.
>
> Start the clock on the deprecation cycle, with an example of how
> to write device subsetting without using -P.
>
> Signed-off-by: Eric Blake
> ---
> qemu-deprecated.texi | 27 +++
> qemu-nbd.texi| 6 --
> qemu-nbd.c | 2 ++
> 3 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index 219206a836f..12f8b30943f 100644
> --- a/qemu-deprecated.texi
> +++ b/qemu-deprecated.texi
> @@ -175,3 +175,30 @@ The above, converted to the current supported format:
> @subsubsection "irq": "" (since 3.0.0)
>
> The ``irq'' property is obsoleted.
> +
> +@section Related binaries
> +
> +@subsection qemu-nbd --partition (since 4.0.0)
> +
> +The ``qemu-nbd --partition $digit'' code (also spelled @option{-P})
> +can only handle MBR partitions, and does not correctly handle logical
> +partitions beyond partition 5. If you know the relative position of
> +the partition (perhaps by using @code{sfdisk} or similar, either in
> +the guest or when mapping the entire device through /dev/nbd0 in the
> +host), you can achieve the effect of exporting just that subset of the
> +disk by use of the @option{--image-opts} option with a raw blockdev
> +using the @code{offset} and @code{size} parameters layered on top of
> +any other existing blockdev.
> +
> +For example, if partition 1 is 100MiB starting at 1MiB, the old command
> +
> +@example{qemu-nbd -P 1 -f qcow2 file.qcow2}
> +
> +can be rewritten as:
> +
> +@example{qemu-nbd --image-opts
> driver=raw,offset=1M,size=100M,file.driver=qcow2,file.backing.driver=file,file.backing.filename=file.qcow2}
> +
> +Alternatively, the @code{nbdkit} project provides a more powerful
> +partition filter on top of its nbd plugin, which can be used to select
> +an arbitrary MBR or GPT partition on top of any other full-image NBD
> +export.
You might want to add the actual command here. Unfortunately nbdkit
cannot read qcow2 files meaning (as you note already) that you have to
forward the connection through the nbdkit-nbd-plugin to qemu-nbd.
This worked for me:
qemu-nbd -t -k /tmp/sock -f qcow2 file.qcow2 &
nbdkit -f --filter=partition nbd socket=/tmp/sock partition=1 &
If you drop the requirement to demonstrate this with qcow2 then the
command would be just this:
nbdkit --filter=partition file disk.raw partition=1
> diff --git a/qemu-nbd.texi b/qemu-nbd.texi
> index 386bece4680..d0c51828149 100644
> --- a/qemu-nbd.texi
> +++ b/qemu-nbd.texi
> @@ -56,8 +56,10 @@ auto-detecting.
> @item -r, --read-only
> Export the disk as read-only.
> @item -P, --partition=@var{num}
> -Only expose MBR partition @var{num}. Understands physical partitions
> -1-4 and logical partitions 5-8.
> +Deprecated: Only expose MBR partition @var{num}. Understands physical
> +partitions 1-4 and logical partition 5. New code should instead use
> +@option{--image-opts} with the raw driver wrapping a subset of the
> +original image.
> @item -B, --bitmap=@var{name}
> If @var{filename} has a qcow2 persistent bitmap @var{name}, expose
> that bitmap via the ``qemu:dirty-bitmap:@var{name}'' context
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 1f7b2a03f5d..00c07fd27ea 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -787,6 +787,8 @@ int main(int argc, char **argv)
> flags &= ~BDRV_O_RDWR;
> break;
> case 'P':
> +warn_report("The '-P' option is deprecated; use --image-opts
> with "
> +"a raw device wrapper for subset exports instead"
Re: [libvirt] [PATCH 1/2] nwfilter: Fix pointer.
On Mon, Jan 21, 2019 at 03:23:59PM +, Daniel P. Berrangé wrote:
> On Mon, Jan 21, 2019 at 03:13:20PM +0000, Richard W.M. Jones wrote:
> > GCC 9 complains:
> >
> > nwfilter/nwfilter_dhcpsnoop.c: In function 'virNWFilterDHCPSnoopThread':
> > nwfilter/nwfilter_dhcpsnoop.c:1456:31: error: converting a packed
> > 'virNWFilterSnoopEthHdrPtr' {aka 'struct _virNWFilterSnoopEthHdr *'}
> > pointer (alignment 1) to 'const u_char *' {aka 'const unsigned char *'}
> > (alignment 8) may result in an unaligned pointer value
> > [-Werror=address-of-packed-member]
> > 1456 | (const u_char **));
> > | ^
>
> I tend to think this warning is bogus. We are not de-referencing
> any packed fields within the sctruct when we call to pcap_next_ex()
> with the cast. pcap_next_ex() is just going to fill the entire
> memory region with a read off the wire, so it would not be triggering
> unaligned access either. IOW, I don't think the compiler should be
> warning there
>
> IIUC gcc X.0.0 versions are not in fact relases, but rather
> pre-release snapshots.
>
> If so, I think this might be a bug that needs reporting against
> the GCC pre-release.
>
>
>
> > nwfilter/nwfilter_dhcpsnoop.c:183:8: note: defined here
> > 183 | struct _virNWFilterSnoopEthHdr {
> > |^~~
> >
> > However it seems like there's more going on here than just an enhanced
> > GCC warning. The function pcap_next_ex is documented as:
> >
> >the pointer pointed to by the
> >pkt_data argument is set to point to the data in the packet
> >
> > We are passing a struct here rather than a pointer. I changed the
> > code to pass a pointer instead.
>
> > diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c
> > b/src/nwfilter/nwfilter_dhcpsnoop.c
> > index 58f0057c3f..45873a542c 100644
> > --- a/src/nwfilter/nwfilter_dhcpsnoop.c
> > +++ b/src/nwfilter/nwfilter_dhcpsnoop.c
> > @@ -1335,7 +1335,7 @@ virNWFilterDHCPSnoopThread(void *req0)
> > {
> > virNWFilterSnoopReqPtr req = req0;
> > struct pcap_pkthdr *hdr;
> > -virNWFilterSnoopEthHdrPtr packet;
> > +const virNWFilterSnoopEthHdrPtr *packetPtr;
>
> virNWFilterSnoopEthHdrPtr is already a pointer to a virNWFilterSnoopEthHdr.
>
> So this change turns it into a pointer to a pointer
Duh you're right there.
Yup as you say this patch is bogus and more likely indicates
some bug in GCC.
Rich.
> > int ifindex = 0;
> > int errcount = 0;
> > int tmp = -1, rv, n, pollTo;
> > @@ -1453,7 +1453,7 @@ virNWFilterDHCPSnoopThread(void *req0)
> > n--;
> >
> > rv = pcap_next_ex(pcapConf[i].handle, ,
> > - (const u_char **));
> > + (const u_char **));
>
> And then into a pointer to a pointer to a pointer.
>
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] nwfilter: Fix pointer.
GCC 9 complains:
nwfilter/nwfilter_dhcpsnoop.c: In function 'virNWFilterDHCPSnoopThread':
nwfilter/nwfilter_dhcpsnoop.c:1456:31: error: converting a packed
'virNWFilterSnoopEthHdrPtr' {aka 'struct _virNWFilterSnoopEthHdr *'} pointer
(alignment 1) to 'const u_char *' {aka 'const unsigned char *'} (alignment 8)
may result in an unaligned pointer value [-Werror=address-of-packed-member]
1456 | (const u_char **));
| ^
nwfilter/nwfilter_dhcpsnoop.c:183:8: note: defined here
183 | struct _virNWFilterSnoopEthHdr {
|^~~
However it seems like there's more going on here than just an enhanced
GCC warning. The function pcap_next_ex is documented as:
the pointer pointed to by the
pkt_data argument is set to point to the data in the packet
We are passing a struct here rather than a pointer. I changed the
code to pass a pointer instead.
---
src/nwfilter/nwfilter_dhcpsnoop.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c
b/src/nwfilter/nwfilter_dhcpsnoop.c
index 58f0057c3f..45873a542c 100644
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -1335,7 +1335,7 @@ virNWFilterDHCPSnoopThread(void *req0)
{
virNWFilterSnoopReqPtr req = req0;
struct pcap_pkthdr *hdr;
-virNWFilterSnoopEthHdrPtr packet;
+const virNWFilterSnoopEthHdrPtr *packetPtr;
int ifindex = 0;
int errcount = 0;
int tmp = -1, rv, n, pollTo;
@@ -1453,7 +1453,7 @@ virNWFilterDHCPSnoopThread(void *req0)
n--;
rv = pcap_next_ex(pcapConf[i].handle, ,
- (const u_char **));
+ (const u_char **));
if (rv < 0) {
/* error reading from socket */
@@ -1530,7 +1530,7 @@ virNWFilterDHCPSnoopThread(void *req0)
continue;
}
-if (virNWFilterSnoopDHCPDecodeJobSubmit(worker, packet,
+if (virNWFilterSnoopDHCPDecodeJobSubmit(worker, *packetPtr,
hdr->caplen,
pcapConf[i].dir,
[i].qCtr) < 0) {
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] A couple of GCC 9 fixes.
I'm not 100% confident about either of these patches, so please take care with them. I also think the second patch is probaby best fixed by disabling the warning, or else a much more fundamental rewrite of the test. Rich. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] tests: Avoid "jump skips variable initialization" with GCC 9.
GCC 9 gives pages of errors like:
qemumonitorjsontest.c: In function 'mymain':
qemumonitorjsontest.c:2904:9: error: jump skips variable initialization
[-Werror=jump-misses-init]
2904 | goto cleanup;
| ^~~~
qemumonitorjsontest.c:3111:2: note: label 'cleanup' defined here
3111 | cleanup:
| ^~~
qemumonitorjsontest.c:2920:54: note: '({anonymous})' declared here
2920 | simpleFunc = (testQemuMonitorJSONSimpleFuncData) {.xmlopt =
driver.xmlopt, \
| ^
qemumonitorjsontest.c:3008:5: note: in expansion of macro 'DO_TEST_GEN'
3008 | DO_TEST_GEN(qemuMonitorJSONBlockdevMediumInsert);
| ^~~
By moving the cleanup section up near the top of the function we can
avoid this. I think a better way might be to disable the warning.
---
tests/qemumonitorjsontest.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 1a8a31717f..299c5f0cbe 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -2900,8 +2900,12 @@ mymain(void)
if (!(qapiData.schema = testQEMUSchemaLoad())) {
VIR_TEST_VERBOSE("failed to load qapi schema\n");
-ret = -1;
-goto cleanup;
+cleanup:
+VIR_FREE(metaschemastr);
+virJSONValueFree(metaschema);
+virHashFree(qapiData.schema);
+qemuTestDriverFree();
+return -1;
}
#define DO_TEST(name) \
@@ -3098,7 +3102,6 @@ mymain(void)
if (!(metaschema = testQEMUSchemaGetLatest()) ||
!(metaschemastr = virJSONValueToString(metaschema, false))) {
VIR_TEST_VERBOSE("failed to load latest qapi schema\n");
-ret = -1;
goto cleanup;
}
@@ -3108,7 +3111,6 @@ mymain(void)
#undef DO_TEST_QAPI_SCHEMA
- cleanup:
VIR_FREE(metaschemastr);
virJSONValueFree(metaschema);
virHashFree(qapiData.schema);
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] ANNOUNCE: libguestfs 1.40 released
I'm pleased to announce libguestfs 1.40, a library and a set of tools for accessing and modifying virtual machine disk images. This release represents about a year of work by many contributors. I'd like to call out in particular substantial contributions from: Pino Toscano, Tomáš Golembiovský, Nir Soffer and Nikolay Ivanets. See the release notes below for full details. You can get libguestfs 1.40 from here: Main website: http://libguestfs.org/ [not updated yet] Source: http://libguestfs.org/download/1.40-stable/ Fedora: https://koji.fedoraproject.org/koji/packageinfo?packageID=8391 Debian/experimental: https://packages.debian.org/libguestfs0 Rich. -- Release notes for libguestfs 1.40 These are also available online at: http://libguestfs.org/guestfs-release-notes.1.html New features New features in existing tools Virt-inspector now displays the libosinfo short ID for guests (Pino Toscano). Guestfish -N will now generate 1G disks instead of 100M disks by default. Virt-resize supports f2fs filesystems (Pino Toscano). libguestfs-test-tool now supports bash tab completion (Pino Toscano). The --machine-readable option now supports sending output to files or stdout/stderr. This works uniformly across most OCaml-based virt tools, specifically: virt-builder, virt-builder-repository, virt-dib, virt-get-kernel, virt-resize, virt-sparsify, and virt-v2v (Pino Toscano). Virt-builder --uninstall option now works on SUSE (Sebastian Meyer). Virt-builder now supports Windows. We are not able to ship Windows templates in the public respository for obvious licensing reasons, but if you are an MSDN subscriber you may build your own. See https://rwmj.wordpress.com/2018/09/13/creating-windows-templates-for-virt-builder/ Many tools now support a --key option allowing you to pass in decryption keys for filesystems stored in local files on the host rather than having to feed them in over stdin (Pino Toscano). virt-v2v and virt-p2v New -o rhv-upload mode for directly uploading images to RHV, bypassing the Export Storage Domain (Tomáš Golembiovský, Nir Soffer, Daniel Erez, Pino Toscano). New -o openstack mode for directly uploading images to OpenStack and Cinder using OpenStack APIs. Virt-v2v now has a general mechanism for input and output options: -io and -oo. Virt-v2v can now install the RHV tools or QEMU GA in guests (Tomáš Golembiovský). The huge manual has now been split into several smaller documents and is more focused on helping users to accomplish specific v2v tasks. Multiple improvements to the OVF metadata when converting to RHV (Tomáš Golembiovský, Pino Toscano, Arik Hadas). Virt-v2v can now convert Linux guests with split kernel packages, especially Ubuntu 18.04 (Pino Toscano). Virt-v2v old --password-file option has been replaced by -ip (the old option remains for backwards compatibility). Virt-v2v now preserves the VM Generation ID. Virt-v2v has a new --mac option allowing specific NICs to be mapped precisely to networks or bridges on the target. New virt-v2v --print-estimate option to estimate the size of data that virt-v2v will copy. Virt-v2v is now usually able to remove open-vm-tools and VMware Tools from the Linux guest during conversion (Pino Toscano). Virt-v2v can now support UEFI conversions to RHV (Tomáš Golembiovský). Virt-p2v now prefers you to shut down instead of rebooting the machine after conversion has finished (Pino Toscano). Virt-v2v now writes the libosinfo short ID to the libvirt metadata when using -o local and -o libvirt output modes (Martin Kletzander). Language bindings OpenJDK 10+ is supported (Pino Toscano). Java bindings fixed for Gentoo host (Martin Kletzander). Inspection Support Kali Linux (Pino Toscano). When inspecting mountpoints, look for /etc/mdadm/mdadm.conf as well as /etc/mdadm.conf (Nikolay Ivanets). Improved support for OpenSUSE Tumbleweed (Pino Toscano). Architectures and platforms Miscellaneous macOS build fixes (Adam Robinson). Other Multiple documentation typos fixed (Yuri Chornoivan). Security See also guestfs-security(1). CVE-2018-11806 Qemu's slirp (userspace networking) had several buffer overflows which could be triggered from the guest or network side. API New APIs "f2fs_expand" Expand an f2fs filesystem (Pino Toscano). "inspect_get_osinfo" Get the libosinfo short ID for the inspected guest (Pino Toscano). "lvm_scan"
Re: [libvirt] [ocaml PATCH] Cast virError* enums to int for comparisons with 0
On Fri, Nov 02, 2018 at 03:52:23PM +0100, Pino Toscano wrote:
> The actual type of an enum in C is implementation defined when there are
> no negative values, and thus it can be int, or uint. This is the case
> of the virError* enums in libvirt, as they do not have negative values.
>
> Hence, to avoid hitting tautological comparison errors when checking
> their rage, temporarly cast the enum values to int when checking they
s/rage/range/
The patch itself is fine:
ACK.
Rich.
> are not negative. The check is there to ensure the value is within the
> range of the OCaml type used to represent it.
>
> Signed-off-by: Pino Toscano
> ---
> libvirt/libvirt_c_epilogue.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/libvirt/libvirt_c_epilogue.c b/libvirt/libvirt_c_epilogue.c
> index 29656a4..4e75d2f 100644
> --- a/libvirt/libvirt_c_epilogue.c
> +++ b/libvirt/libvirt_c_epilogue.c
> @@ -153,7 +153,7 @@ Val_err_number (virErrorNumber code)
>CAMLparam0 ();
>CAMLlocal1 (rv);
>
> - if (0 <= code && code <= MAX_VIR_CODE)
> + if (0 <= (int) code && code <= MAX_VIR_CODE)
> rv = Val_int (code);
>else {
> rv = caml_alloc (1, 0); /* VIR_ERR_UNKNOWN (code) */
> @@ -169,7 +169,7 @@ Val_err_domain (virErrorDomain code)
>CAMLparam0 ();
>CAMLlocal1 (rv);
>
> - if (0 <= code && code <= MAX_VIR_DOMAIN)
> + if (0 <= (int) code && code <= MAX_VIR_DOMAIN)
> rv = Val_int (code);
>else {
> rv = caml_alloc (1, 0); /* VIR_FROM_UNKNOWN (code) */
> @@ -185,7 +185,7 @@ Val_err_level (virErrorLevel code)
>CAMLparam0 ();
>CAMLlocal1 (rv);
>
> - if (0 <= code && code <= MAX_VIR_LEVEL)
> + if (0 <= (int) code && code <= MAX_VIR_LEVEL)
> rv = Val_int (code);
>else {
> rv = caml_alloc (1, 0); /* VIR_ERR_UNKNOWN_LEVEL (code) */
> --
> 2.17.2
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ocaml PATCH] doc: invoke ocamldoc with -colorize-code
On Mon, Oct 15, 2018 at 04:07:01PM +0200, Pino Toscano wrote: > This way, the OCaml snippets are colorized. > > The OCaml version required is higher than the first version shipping > ocamldoc with this option, so that can be done unconditionally. > > Signed-off-by: Pino Toscano > --- > Makefile.in | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile.in b/Makefile.in > index ad5a036..f119dbc 100644 > --- a/Makefile.in > +++ b/Makefile.in > @@ -23,7 +23,7 @@ INSTALL = @INSTALL@ > MAKENSIS = @MAKENSIS@ > > OCAMLDOC= @OCAMLDOC@ > -OCAMLDOCFLAGS:= -html -sort > +OCAMLDOCFLAGS:= -html -sort -colorize-code > > SUBDIRS = libvirt examples Trivial improvement, ACK. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ocaml PATCH 0/4] Misc build improvements
On Mon, Oct 15, 2018 at 04:02:31PM +0200, Pino Toscano wrote: > A round of few build system improvements: > - use the OCaml build macros from libguestfs > - remove generated stuff > - use pkg-config to find & use libvirt > > Pino Toscano (4): > build: move OCaml macros to a m4 subdir > build: sync OCaml macros from libguestfs > build: remove config.h.in > build: use pkg-config to find libvirt > > .gitignore | 2 + > aclocal.m4 | 170 -- > config.h.in | 61 - > configure.ac| 26 +- > libvirt/Makefile.in | 14 +-- > m4/ocaml.m4 | 217 > 6 files changed, 232 insertions(+), 258 deletions(-) > delete mode 100644 aclocal.m4 > delete mode 100644 config.h.in > create mode 100644 m4/ocaml.m4 Looks fine, ACK series. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ocaml] reset and resync the libvirt-ocaml repository
On Thu, Sep 06, 2018 at 05:13:23PM +0200, Pino Toscano wrote: > Hi, > > for reasons mostly lost in the history, after the libvirt-ocaml > repository was converted to git, it was not used by its main author > (Rich Jones); the development continued on Rich's git, at > http://git.annexia.org/?p=ocaml-libvirt.git;a=summary > > After a talk with Rich, we agreed that it was better to move the > development back to libvirt.org, just like all the other bindings. > There are two problems however: > > 1) the first 38 commits have an bad author/committer date, and this is >also the reason why the existing libvirt-ocaml is not mirrored on >github > > 2) the top 3 commits on libvirt-ocaml were not integrated back to >Rich's ocaml-libvirt, and maybe their content might not be totally >OK (I will let Rich comment more on this) > > While rewriting history is bad, > - most probably there are not many users of libvirt-ocaml around, > - the repository itself is very small (< 500k), > - in general it will better to have a working repository > > So what I'm proposing is to replace the libvirt-ocaml repository with > a fixed version of Rich's ocaml-libvirt, and directly on the git hosting > side (i.e. not using force-push on the current one). Rich has already > commit access for libvirt, so there are no problems to keep his > maintainer role on it. Once done, we can notify users in this list > about it. > > What do you think? Is it an acceptable path forward? I had a chat with Pino offline and I agree with this plan. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Introduce 16550A serial console model
On Mon, Aug 27, 2018 at 02:10:18PM +0200, Andrea Bolognani wrote:
> None of the existing models is suitable for use with
> RISC-V virt guests, and we don't want information about
> the serial console to be missing from the XML.
>
> The name is based on comments in qemu/hw/riscv/virt.c:
>
> RISC-V machine with 16550a UART and VirtIO MMIO
>
> and in qemu/hw/char/serial.c:
>
> QEMU 16550A UART emulation
>
> along with the output of dmesg in the guest:
>
> Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
> 1000.uart: ttyS0 at MMIO 0x1000 (irq = 13,
> base_baud= 230400) is a 16550A
FWIW on the real hardware:
[4.078734] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
[4.084078] 1001.serial: ttySI0 at MMIO 0x1001 (irq = 43, base_baud
= 0) is a sifive-serial
[4.751449] 10011000.serial: ttySI1 at MMIO 0x10011000 (irq = 44, base_baud
= 0) is a sifive-serial
Now about the patch ...
I think this is fine provided it doesn't bake in future libvirt API
that we'll regret.
However the real story is that what real RISC-V hardware will look
like is undecided. For embedded they're making all the same mistakes
as ARMv7 all over again (despite our clear warnings). So expect
wildly different implementations, no way to probe at runtime, random
device trees, crazy board descriptions littering the kernel and qemu,
out of tree drivers, etc. All that crap.
For servers there's a working group supposedly looking into this and
going to produce an SBSA/SBBR-style specification. Last time I looked
it was a single page with no detail, and I can't actually find the
link to that right now. In any case it's nowhere near decided. It
would be nice if they standardized a 16550A UART at a known address,
PCIe, etc. but at the moment I wouldn't make plans based on sanity
prevailing.
TL;DR: Patch is fine as long as we can change things later without
maintaining ABI.
Rich.
> Signed-off-by: Andrea Bolognani
> ---
> CC'ing Rich mostly so that he can double-check the name
> choice is sensible.
>
> docs/formatdomain.html.in | 12 +-
> docs/schemas/domaincommon.rng | 1 +
> src/conf/domain_conf.c| 1 +
> src/conf/domain_conf.h| 1 +
> src/qemu/qemu_command.c | 12 ++
> src/qemu/qemu_domain.c| 27 ---
> tests/qemuxml2xmloutdata/riscv64-virt.xml | 4 +++-
> 7 files changed, 44 insertions(+), 14 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 0cbf570a13..eb619a1656 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -7011,7 +7011,8 @@ qemu-kvm -net nic,model=? /dev/null
>is available) and pci-serial (usable whenever PCI support
>is available); since 3.10.0,
>spapr-vio-serial (usable with ppc64/pseries guests),
> - system-serial (usable with aarch64/virt guests) and
> + system-serial (usable with aarch64/virt and,
> + since 4.7.0, riscv/virt guests) and
>sclp-serial (usable with s390 and s390x guests) are
>available as well.
>
> @@ -7025,10 +7026,11 @@ qemu-kvm -net nic,model=? /dev/null
>target type); pci-serial
>(usable with the pci-serial target type);
>spapr-vty (usable with the spapr-vio-serial
> - target type); pl011 (usable with the
> - system-serial target type); sclpconsole and
> - sclplmconsole (usable with the sclp-serial
> - target type).
> + target type); pl011 and,
> + since 4.7.0, 16550a (usable
> + with the system-serial target type);
> + sclpconsole and sclplmconsole (usable with
> + the sclp-serial target type).
>
>
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index f176538195..3796eb4b5e 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3733,6 +3733,7 @@
>pci-serial
>spapr-vty
>pl011
> + 16550a
>sclpconsole
>sclplmconsole
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index bde9fef914..8af59bb4bb 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -493,6 +493,7 @@ VIR_ENUM_IMPL(virDomainChrSerialTargetModel,
>"pl011",
>"sclpconsole",
>"sclplmconsole",
> + "16550a",
> );
>
> VIR_ENUM_IMPL(virDomainChrDevice, VIR_DOMAIN_CHR_DEVICE_TYPE_LAST,
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index c0ad072db5..8a3673361a 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1133,6 +1133,7 @@ typedef enum {
> VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_PL011,
> VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_SCLPCONSOLE,
> VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_SCLPLMCONSOLE,
> +VIR_DOMAIN_CHR_SERIAL_TARGET_MODEL_16550A,
>
[libvirt] [PATCH] spec: Add support for RISC-V (riscv64)
From: David Abdurachmanov
---
libvirt.spec.in | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index e7196b7d3b..5cb995a6a4 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -101,7 +101,7 @@
%endif
# Numactl is not available on s390[x] and ARM
-%ifarch s390 s390x %{arm}
+%ifarch s390 s390x %{arm} riscv64
%define with_numactl 0
%endif
@@ -120,7 +120,7 @@
%endif
# zfs-fuse is not available on some architectures
-%ifarch s390 s390x aarch64
+%ifarch s390 s390x aarch64 riscv64
%define with_storage_zfs 0
%endif
@@ -195,7 +195,7 @@
%if %{with_qemu} || %{with_lxc} || %{with_uml}
# numad is used to manage the CPU and memory placement dynamically,
# it's not available on s390[x] and ARM.
-%ifnarch s390 s390x %{arm}
+%ifnarch s390 s390x %{arm} riscv64
%define with_numad0%{!?_without_numad:1}
%endif
%endif
--
2.18.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] FYI: libvirt package on Fedora RISC-V
On Mon, Aug 20, 2018 at 10:40:42AM +0200, Andrea Bolognani wrote: > On Sun, 2018-08-19 at 19:46 +0100, Richard W.M. Jones wrote: > > On Sun, Aug 19, 2018 at 07:44:06PM +0100, Richard W.M. Jones wrote: > > > > > > http://fedora-riscv.tranquillity.se/koji/taskinfo?taskID=95457 > > Neat! > > > > There were some minor changes to the spec file: > > > > > > https://src.fedoraproject.org/rpms/libvirt/c/18f7b8c79c2876b2d3a6ebe2279fd6526321536b?branch=master > > > > I should of course say for clarity that David has only compiled the > > libvirt package so it can be used for other packages that BR it. > > The spec file changes look like they could be merged upstream > right away, can he (or you) submit them? Right, sorry, I forgot that we're maintaining the spec file in the libvirt sources. I'll post a patch soon. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] FYI: libvirt package on Fedora RISC-V
On Sun, Aug 19, 2018 at 07:44:06PM +0100, Richard W.M. Jones wrote: > > http://fedora-riscv.tranquillity.se/koji/taskinfo?taskID=95457 > > There were some minor changes to the spec file: > > https://src.fedoraproject.org/rpms/libvirt/c/18f7b8c79c2876b2d3a6ebe2279fd6526321536b?branch=master I should of course say for clarity that David has only compiled the libvirt package so it can be used for other packages that BR it. This does not include any of the proposed patches to add support for qemu-system-riscv64. And there's no hardware virt support on any existing RISC-V systems. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] FYI: libvirt package on Fedora RISC-V
http://fedora-riscv.tranquillity.se/koji/taskinfo?taskID=95457 There were some minor changes to the spec file: https://src.fedoraproject.org/rpms/libvirt/c/18f7b8c79c2876b2d3a6ebe2279fd6526321536b?branch=master Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix UNIX socket chardevs operating in client mode
On Fri, Jul 06, 2018 at 11:03:00AM +0100, Daniel P. Berrangé wrote:
> When support was adding for passing a pre-opened listener socket to UNIX
> chardevs, it accidentally passed the listener socket for client mode
> chardevs too with predictable amounts of fail resulting.
>
> Expand the unit test coverage to validate that we are only doing FD
> passing when operating in server mode.
>
> Reported-by: Richard W.M. Jones
> Signed-off-by: Daniel P. Berrangé
I suppose you might want to mention in the commit message that the bug
only affects qemu 2.12, and the bug number (1598440).
Anyhow, I tested this patch and it fixes the problem, so:
Tested-by: Richard W.M. Jones
There is a scratch build for Rawhide containing the patch here:
https://koji.fedoraproject.org/koji/taskinfo?taskID=28050872
Note that the patch requires quite a bit of adjustment to apply and
build against 4.5.0. I wasn't at all confident that I've done it
right so I didn't push anything to Rawhide. It'd be good to have this
fix in Rawhide & RHEL 7 asap though.
Rich.
> src/qemu/qemu_command.c | 3 +-
> .../qemuxml2argvdata/serial-unix-chardev.args | 2 ++
> .../serial-unix-chardev.x86_64-latest.args| 36 +++
> .../qemuxml2argvdata/serial-unix-chardev.xml | 4 +++
> tests/qemuxml2argvtest.c | 1 +
> 5 files changed, 45 insertions(+), 1 deletion(-)
> create mode 100644
> tests/qemuxml2argvdata/serial-unix-chardev.x86_64-latest.args
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 82d8030a33..32eb59b6ab 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -5083,7 +5083,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
> break;
>
> case VIR_DOMAIN_CHR_TYPE_UNIX:
> -if ((flags & QEMU_BUILD_CHARDEV_UNIX_FD_PASS) &&
> +if (dev->data.nix.listen &&
> +(flags & QEMU_BUILD_CHARDEV_UNIX_FD_PASS) &&
> virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS)) {
> if (qemuSecuritySetSocketLabel(secManager, (virDomainDefPtr)def)
> < 0)
> goto cleanup;
> diff --git a/tests/qemuxml2argvdata/serial-unix-chardev.args
> b/tests/qemuxml2argvdata/serial-unix-chardev.args
> index 584f4a1dd1..873d3263c6 100644
> --- a/tests/qemuxml2argvdata/serial-unix-chardev.args
> +++ b/tests/qemuxml2argvdata/serial-unix-chardev.args
> @@ -26,4 +26,6 @@ server,nowait \
> -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
> -chardev socket,id=charserial0,path=/tmp/serial.sock \
> -device isa-serial,chardev=charserial0,id=serial0 \
> +-chardev socket,id=charserial1,path=/tmp/serial-server.sock,server,nowait \
> +-device isa-serial,chardev=charserial1,id=serial1 \
> -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> diff --git a/tests/qemuxml2argvdata/serial-unix-chardev.x86_64-latest.args
> b/tests/qemuxml2argvdata/serial-unix-chardev.x86_64-latest.args
> new file mode 100644
> index 00..ce7a7f80d7
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/serial-unix-chardev.x86_64-latest.args
> @@ -0,0 +1,36 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/home/test \
> +USER=test \
> +LOGNAME=test \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu-system-i686 \
> +-name guest=QEMUGuest1,debug-threads=on \
> +-S \
> +-object secret,id=masterKey0,format=raw,\
> +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
> +-machine pc,accel=tcg,usb=off,dump-guest-core=off \
> +-m 214 \
> +-realtime mlock=off \
> +-smp 1,sockets=1,cores=1,threads=1 \
> +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> +-display none \
> +-no-user-config \
> +-nodefaults \
> +-chardev socket,id=charmonitor,fd=1729,server,nowait \
> +-mon chardev=charmonitor,id=monitor,mode=control \
> +-rtc base=utc \
> +-no-shutdown \
> +-no-acpi \
> +-boot strict=on \
> +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
> +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
> +-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1
> \
> +-chardev socket,id=charserial0,path=/tmp/serial.sock \
> +-device isa-serial,chardev=charserial0,id=serial0 \
> +-chardev socket,id=charserial1,fd=1729,server,nowait \
> +-device isa-serial,chardev=charserial1,id=serial1 \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \
> +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
> +resourcecontrol=deny \
> +-msg timestamp=on
> diff --git a/tests/qemuxml2argvdata/serial-unix-chardev.xml
> b/tests/qemuxml2argvdata/serial-unix-chardev.xml
> index 04f83779ce..af513d6445 100644
> --- a/tests/qemuxml2argvdata/serial-unix-char
Re: [libvirt] Mystery of qemu being unable to open NBD Unix domain socket in current directory
On Thu, Jun 07, 2018 at 01:38:47PM +0200, Peter Krempa wrote: > On Thu, Jun 07, 2018 at 12:20:16 +0100, Richard W.M. Jones wrote: > > > > I don't know whether or not we decided this was a bug, but > > I have filed one anyway: > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1588447 > > Your XML does not conform to the libvirt domain XML schema which always > called for an absolute path as we do with all paths in the domain XML: > > > > unix > > > > > > > > We just did not validate this particular code path in the parser itself > (when the domain schema validation is not used). Sure. Libvirt does need to validate the paths because it's possible that the *wrong* path will be opened if a relative path is used, eg. if a socket with the same name happens to exist in "/" (or whatever is the working directory that libvirt changes to). Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Mystery of qemu being unable to open NBD Unix domain socket in current directory
I don't know whether or not we decided this was a bug, but I have filed one anyway: https://bugzilla.redhat.com/show_bug.cgi?id=1588447 Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Mystery of qemu being unable to open NBD Unix domain socket in current directory
On Wed, Jun 06, 2018 at 01:25:27PM -0500, Eric Blake wrote: > On 06/06/2018 12:29 PM, Richard W.M. Jones wrote: > >I'm chasing down a very frustrating bug which only happens on i686 & > >Koji during the nbdkit tests and seemingly nowhere else. Anyway this > >is what I've been able to put together: > > > >The libguestfs appliance (guest) is created with this XML snippet: > > > > > > > > > > Have you tried an absolute path? Yes it works with an absolute path. Isn't this a bug in libvirt? Also it works on x86_64 and in earlier versions of libvirt. I'll post my nbdkit workaround soon. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
