February 2019 - Devel - libvirt List Archives

[libvirt] [PATCH] util: remove test code accidentally committed to virFirewallDZoneExists
by Laine Stump 03 Feb '19

03 Feb '19

Just before pushing the series containing commit 3bba4825 I had added a "return true" to the top of virFirewallDZoneExists() to measure the impact of calling that function once per network during startup. I found that the effect was minimal, but forgot to remove the "return true" before pushing. This unfortunately causes a failure to start networks on systems that have a firewalld version that doesn't support our libvirt zone file (i.e. pretty much everyone). This patch removes the unintended line. Signed-off-by: Laine Stump <laine(a)laine.org> --- Pushed as trivial / buildbreaker. src/util/virfirewalld.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/util/virfirewalld.c b/src/util/virfirewalld.c index ca03020f74..47bacdcf4a 100644 --- a/src/util/virfirewalld.c +++ b/src/util/virfirewalld.c @@ -235,8 +235,6 @@ virFirewallDZoneExists(const char *match) char **zones = NULL; bool result = false; - return true; - if (virFirewallDGetZones(&zones, &nzones) < 0) goto cleanup; -- 2.20.1

1 0

[libvirt] [PATCH 0/3] bhyve: bhyve:commandline followup fixes
by Roman Bogorodskiy 01 Feb '19

01 Feb '19

Roman Bogorodskiy (3): bhyve: bhyveDomainDefNamespaceFormatXML cleanup bhyve: emit warning when using bhyve:commandline docs: bhyve: warn about bhyve:commandline risks docs/drvbhyve.html.in | 5 +++++ src/bhyve/bhyve_command.c | 4 ++++ src/bhyve/bhyve_domain.c | 4 ++-- 3 files changed, 11 insertions(+), 2 deletions(-) -- 2.20.1

2 5

[libvirt] [PATCH v2] qemu-nbd: Deprecate qemu-nbd --partition
by Eric Blake 01 Feb '19

01 Feb '19

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 <eblake(a)redhat.com> --- 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 deprecated; use --image-opts with " + "a raw device wrapper for subset exports instead"); if (qemu_strtoi(optarg, NULL, 0, &partition) < 0 || partition < 1 || partition > 8) { error_report("Invalid partition '%s'", optarg); -- 2.20.1

3 3

[libvirt] [PATCH] qemu: command: Don't skip 'readonly' and throttling info for empty drive
by Peter Krempa 01 Feb '19

01 Feb '19

In commit f80eae8c2ae I was too agresive in removing properties of -drive for empty drives. It turns out that qemu actually persists the state of 'readonly' and the throttling information even for the empty drive. Removing 'readonly' thus made qemu open any subsequent images added via the 'change' command as RW which was forbidden by selinux thanks to the restrictive sVirt label for readonly media. Fix this by formating the property again and bump the tests and leave a note detailing why the rest of the properties needs to be skipped. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/qemu/qemu_command.c | 18 +++++++++++++----- tests/qemuxml2argvdata/disk-cdrom.args | 4 ++-- .../disk-cdrom.x86_64-2.12.0.args | 4 ++-- .../disk-cdrom.x86_64-latest.args | 4 ++-- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a59583fb75..6d3aa69569 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1764,10 +1764,18 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, } } + if (disk->src->readonly) + virBufferAddLit(&opt, ",readonly=on"); + + /* qemu rejects some parameters for an empty -drive, so we need to skip + * them in that case: + * cache: modifies properties of the format driver which is not present + * copy_on_read: really only works for floppies + * discard: modifies properties of format driver + * detect_zeroes: works but really depends on discard so it's useless + * iomode: setting it to 'native' requires a specific cache mode + */ if (!virStorageSourceIsEmpty(disk->src)) { - if (disk->src->readonly) - virBufferAddLit(&opt, ",readonly=on"); - if (disk->cachemode) { virBufferAsprintf(&opt, ",cache=%s", qemuDiskCacheV2TypeToString(disk->cachemode)); @@ -1792,10 +1800,10 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, virBufferAsprintf(&opt, ",aio=%s", virDomainDiskIoTypeToString(disk->iomode)); } - - qemuBuildDiskThrottling(disk, &opt); } + qemuBuildDiskThrottling(disk, &opt); + if (virBufferCheckError(&opt) < 0) goto error; diff --git a/tests/qemuxml2argvdata/disk-cdrom.args b/tests/qemuxml2argvdata/disk-cdrom.args index a9f60aa477..4823ae82de 100644 --- a/tests/qemuxml2argvdata/disk-cdrom.args +++ b/tests/qemuxml2argvdata/disk-cdrom.args @@ -27,7 +27,7 @@ bootindex=1 \ -drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-0-1,media=cdrom,\ readonly=on \ -device ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \ --drive if=none,id=drive-ide0-1-0,media=cdrom \ +-drive if=none,id=drive-ide0-1-0,media=cdrom,readonly=on \ -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \ --drive if=none,id=drive-ide0-1-1,media=cdrom \ +-drive if=none,id=drive-ide0-1-1,media=cdrom,readonly=on \ -device ide-drive,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1 diff --git a/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args b/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args index a39d920f67..2fe84177b8 100644 --- a/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args +++ b/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args @@ -28,10 +28,10 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ -drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-0-1,readonly=on \ -device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \ --drive if=none,id=drive-ide0-1-0 \ +-drive if=none,id=drive-ide0-1-0,readonly=on \ -device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0,\ write-cache=on \ --drive if=none,id=drive-ide0-1-1 \ +-drive if=none,id=drive-ide0-1-1,readonly=on \ -device ide-cd,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1 \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ resourcecontrol=deny \ diff --git a/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args b/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args index 029ae23dfa..9b9451f435 100644 --- a/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args +++ b/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args @@ -28,10 +28,10 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ -drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-0-1,readonly=on \ -device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \ --drive if=none,id=drive-ide0-1-0 \ +-drive if=none,id=drive-ide0-1-0,readonly=on \ -device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0,\ write-cache=on \ --drive if=none,id=drive-ide0-1-1 \ +-drive if=none,id=drive-ide0-1-1,readonly=on \ -device ide-cd,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1 \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ resourcecontrol=deny \ -- 2.20.1

2 1

[libvirt] [PATCH] util: Fix build issue with virStorageFileGetNPIVKey
by John Ferlan 01 Feb '19

01 Feb '19

Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- pushed as build breaker <sigh>... src/util/virstoragefile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index d83d84fcf5..6df0885669 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1564,7 +1564,7 @@ virStorageFileGetNPIVKey(const char *path, return ret; } #else -int virStorageFileGetNPIVKey(const char *path, +int virStorageFileGetNPIVKey(const char *path ATTRIBUTE_UNUSED, char **key ATTRIBUTE_UNUSED) { return -1; -- 2.20.1

1 0

[libvirt] [PATCH 0/3] qemu: Improve handling of format for 'network' and 'volume' disks (blockdev-add saga)
by Peter Krempa 01 Feb '19

01 Feb '19

Peter Krempa (3): tests: qemu: Test network disks without format specified explicitly qemu: domain: Assume 'raw' default storage format also for network storage qemu: domain: Treat 'volume' disks as 'raw' if neiter user nor pool provided format src/qemu/qemu_domain.c | 8 ++++++-- tests/qemuxml2argvdata/disk-network-gluster.xml | 2 +- tests/qemuxml2argvdata/disk-network-iscsi.xml | 2 +- tests/qemuxml2argvdata/disk-network-nbd.xml | 2 +- tests/qemuxml2argvdata/disk-source-pool-mode.args | 6 +++--- tests/qemuxml2argvdata/disk-source-pool.args | 4 ++-- 6 files changed, 14 insertions(+), 10 deletions(-) -- 2.20.1

3 5

[libvirt] [PATCH] docs: news: Update the release notes with the SEV permission fix
by Erik Skultety 01 Feb '19

01 Feb '19

Signed-off-by: Erik Skultety <eskultet(a)redhat.com> --- docs/news.xml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 55d6a3926b..fcc42698b3 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -50,6 +50,18 @@ <section title="Improvements"> </section> <section title="Bug fixes"> + <change> + <summary> + qemu: Use CAP_DAC_OVERRIDE during QEMU capabilities probing + </summary> + <description> + By default, libvirt runs the QEMU process as qemu:qemu which could + cause issues during probing as some features (like AMD SEV) might be + inaccesible to QEMU because of file system permissions. Therefore, + CAP_DAC_OVERRIDE is granted to overcome these for the purposes of + probing. + </description> + </change> <change> <summary> storage: Add default mount options for fs/netfs storage pools -- 2.20.1

2 2

[libvirt] [PATCH v2 0/7] network: fix networking for firewalld+nftables
by Laine Stump 01 Feb '19

01 Feb '19

Resolves: https://bugzilla.redhat.com/1638342 Creates-and-Resolves: https://bugzilla.redhat.com/1650320 V1: https://www.redhat.com/archives/libvir-list/2019-January/msg00227.html The detailed explanation of this is in Patch 4/7 and 5/7. Basically, when firewalld enables their new nftables backend, libvirt virtual networks lose all ability to forward packets from guests out to the physical network, and can only communicate with the host itself as much as firewalld's "public" zone will allow (which isn't much, and doesn't include DHCP or DNS). I *think* I've addressed everything in Daniel and John's review comments. In particular, I've made installation of the libvirt zone file optional, and if the libvirt zone is missing, I only log an error if the firewalld backend is set to nftables. Laine Stump (7): configure: change HAVE_FIREWALLD to WITH_FIREWALLD util: move all firewalld-specific stuff into its own files util: new virFirewallD APIs + docs configure: selectively install a firewalld 'libvirt' zone network: set firewalld zone of bridges to "libvirt" zone when appropriate network: allow configuring firewalld zone for virtual network bridge device docs: update news.xml for firewalld zone changes configure.ac | 3 + docs/firewall.html.in | 38 +++ docs/formatnetwork.html.in | 17 + docs/news.xml | 40 +++ docs/schemas/basictypes.rng | 6 + docs/schemas/network.rng | 6 + include/libvirt/virterror.h | 1 + libvirt.spec.in | 31 ++ m4/virt-firewalld-zone.m4 | 45 +++ m4/virt-firewalld.m4 | 4 +- src/conf/network_conf.c | 14 +- src/conf/network_conf.h | 1 + src/libvirt_private.syms | 10 + src/network/Makefile.inc.am | 10 +- src/network/bridge_driver.c | 6 +- src/network/bridge_driver_linux.c | 67 ++++ src/network/libvirt.zone | 23 ++ src/nwfilter/nwfilter_driver.c | 6 +- src/util/Makefile.inc.am | 3 + src/util/virerror.c | 3 +- src/util/virfirewall.c | 86 +---- src/util/virfirewalld.c | 373 +++++++++++++++++++++ src/util/virfirewalld.h | 46 +++ src/util/virfirewalldpriv.h | 30 ++ src/util/virfirewallpriv.h | 2 - tests/networkxml2xmlin/routed-network.xml | 2 +- tests/networkxml2xmlout/routed-network.xml | 2 +- tests/virfirewalltest.c | 2 + 28 files changed, 779 insertions(+), 98 deletions(-) create mode 100644 m4/virt-firewalld-zone.m4 create mode 100644 src/network/libvirt.zone create mode 100644 src/util/virfirewalld.c create mode 100644 src/util/virfirewalld.h create mode 100644 src/util/virfirewalldpriv.h -- 2.20.1

3 23

[libvirt] [PATCH v2 0/4] storage: More uniquely identify NPIV LUNs
by John Ferlan 01 Feb '19

01 Feb '19

v1: https://www.redhat.com/archives/libvir-list/2018-December/msg00562.html but no review until January, see: https://www.redhat.com/archives/libvir-list/2019-January/msg00079.html Changes since v1: * Rework code to have virStorageBackendSCSISerial use the existing virStorageFileGetSCSIKey * Then introduce and use virStorageFileGetNPIVKey in order to get a more unique key for NPIV LUNs. Follows same fallback policy as SCSI LUNs if the called *Key function either doesn't exist or fails to return a valid value. John Ferlan (4): util: Modify virStorageFileGetSCSIKey return storage: Rework virStorageBackendSCSISerial util: Introduce virStorageFileGetNPIVKey storage: Fetch a unique key for vHBA/NPIV LUNs src/libvirt_private.syms | 1 + src/storage/storage_util.c | 45 +++++++---------- src/util/virstoragefile.c | 101 +++++++++++++++++++++++++++++++++++-- src/util/virstoragefile.h | 2 + 4 files changed, 118 insertions(+), 31 deletions(-) -- 2.20.1

3 14

[libvirt] [PATCH] tests: Update qemucaps2xml for QEMU 4.0.0 on x86_64
by Andrea Bolognani 01 Feb '19

01 Feb '19

Commit fb0d0d6c5492 added capabilities data and updated qemucapabilitiestest but forgot to update qemucaps2xmltest at the same time. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- *cough* and of course the reviewer didn't notice *cough* .../qemucaps2xmloutdata/caps_4.0.0.x86_64.xml | 28 +++++++++++++++++++ tests/qemucaps2xmltest.c | 1 + 2 files changed, 29 insertions(+) create mode 100644 tests/qemucaps2xmloutdata/caps_4.0.0.x86_64.xml diff --git a/tests/qemucaps2xmloutdata/caps_4.0.0.x86_64.xml b/tests/qemucaps2xmloutdata/caps_4.0.0.x86_64.xml new file mode 100644 index 0000000000..d41693a001 --- /dev/null +++ b/tests/qemucaps2xmloutdata/caps_4.0.0.x86_64.xml @@ -0,0 +1,28 @@ +<capabilities> + + <host> + <cpu> + <arch>x86_64</arch> + </cpu> + <power_management/> + <iommu support='no'/> + </host> + + <guest> + <os_type>hvm</os_type> + <arch name='x86_64'> + <wordsize>64</wordsize> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <domain type='qemu'/> + <domain type='kvm'/> + </arch> + <features> + <cpuselection/> + <deviceboot/> + <disksnapshot default='on' toggle='no'/> + <acpi default='on' toggle='yes'/> + <apic default='on' toggle='no'/> + </features> + </guest> + +</capabilities> diff --git a/tests/qemucaps2xmltest.c b/tests/qemucaps2xmltest.c index dd9fc6917a..9d6b6f474c 100644 --- a/tests/qemucaps2xmltest.c +++ b/tests/qemucaps2xmltest.c @@ -198,6 +198,7 @@ mymain(void) DO_TEST("x86_64", "caps_2.12.0"); DO_TEST("x86_64", "caps_3.0.0"); DO_TEST("x86_64", "caps_3.1.0"); + DO_TEST("x86_64", "caps_4.0.0"); DO_TEST("aarch64", "caps_2.6.0"); DO_TEST("aarch64", "caps_2.10.0"); DO_TEST("aarch64", "caps_2.12.0"); -- 2.20.1

2 2