- Devel - libvirt List Archives

[libvirt] [PATCH v2] qemu: Regenerate VNC socket paths
by Martin Kletzander 28 Apr '16

28 Apr '16

Similarly to what commit 714080791778 did with some internal paths, clear vnc socket paths that were generated by us. Having such path in the definition can cause trouble when restoring the domain. The path is generated to the per-domain directory that contains the domain ID. However, that ID will be different upon restoration, so qemu won't be able to create that socket because the directory will not be prepared. To be able to migrate to older libvirt, skip formatting the socket path in migratable XML if it was autogenerated. Best viewed with '-C'. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1326270 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- v2: - Be able to migrate to older libvirt as well v1: - https://www.redhat.com/archives/libvir-list/2016-April/msg01735.html src/conf/domain_conf.c | 6 +++- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 11 +++--- src/qemu/qemu_domain.c | 21 +++++++++++- .../qemuxml2argv-graphics-vnc-autosocket.args | 22 ++++++++++++ .../qemuxml2argv-graphics-vnc-autosocket.xml | 34 +++++++++++++++++++ .../qemuxml2xmlout-graphics-vnc-autosocket.xml | 39 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 7 ++++ 8 files changed, 135 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-autosocket.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b46cd2a64f75..d8bed670f243 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21342,7 +21342,11 @@ virDomainGraphicsDefFormat(virBufferPtr buf, switch (def->type) { case VIR_DOMAIN_GRAPHICS_TYPE_VNC: if (def->data.vnc.socket) { - virBufferEscapeString(buf, " socket='%s'", def->data.vnc.socket); + if (!def->data.vnc.socketAutogenerated || + !(flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE)) { + virBufferEscapeString(buf, " socket='%s'", + def->data.vnc.socket); + } } else { if (def->data.vnc.port && (!def->data.vnc.autoport || !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ee66e6d6aa45..31e7a86d4746 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1564,6 +1564,7 @@ struct _virDomainGraphicsDef { bool autoport; char *keymap; char *socket; + bool socketAutogenerated; virDomainGraphicsAuthDef auth; int sharePolicy; } vnc; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9597b307e9de..6fbfe215bf29 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7277,10 +7277,13 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, } if (graphics->data.vnc.socket || cfg->vncAutoUnixSocket) { - if (!graphics->data.vnc.socket && - virAsprintf(&graphics->data.vnc.socket, - "%s/vnc.sock", domainLibDir) == -1) - goto error; + if (!graphics->data.vnc.socket) { + if (virAsprintf(&graphics->data.vnc.socket, + "%s/vnc.sock", domainLibDir) < 0) + goto error; + + graphics->data.vnc.socketAutogenerated = true; + } virBufferAsprintf(&opt, "unix:%s", graphics->data.vnc.socket); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a2f981043915..d6f704d6f91b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1659,10 +1659,26 @@ qemuCanonicalizeMachine(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) } +static void +qemuDomainCleanupInternalPaths(virDomainDefPtr def, virQEMUDriverConfigPtr cfg) +{ + size_t i = 0; + + for (i = 0; i < def->ngraphics; ++i) { + virDomainGraphicsDefPtr graphics = def->graphics[i]; + + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + graphics->data.vnc.socket && + STRPREFIX(graphics->data.vnc.socket, cfg->libDir)) + VIR_FREE(graphics->data.vnc.socket); + } +} + + static int qemuDomainDefPostParse(virDomainDefPtr def, virCapsPtr caps, - unsigned int parseFlags ATTRIBUTE_UNUSED, + unsigned int parseFlags, void *opaque) { virQEMUDriverPtr driver = opaque; @@ -1708,6 +1724,9 @@ qemuDomainDefPostParse(virDomainDefPtr def, qemuDomainDefEnableDefaultFeatures(def); + if (parseFlags & VIR_DOMAIN_DEF_PARSE_INACTIVE) + qemuDomainCleanupInternalPaths(def, cfg); + if (virSecurityManagerVerify(driver->securityManager, def) < 0) goto cleanup; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.args new file mode 100644 index 000000000000..7e1fb6b3717f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.args @@ -0,0 +1,22 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-vnc unix:/tmp/lib/domain--1-QEMUGuest1/vnc.socket \ +-vga cirrus diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.xml new file mode 100644 index 000000000000..fa59c39873a5 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.xml @@ -0,0 +1,34 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' socket='/tmp/lib/domain-99-QEMUGuest1/delete.this.socket'/> + <video> + <model type='cirrus' vram='16384' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-autosocket.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-autosocket.xml new file mode 100644 index 000000000000..744053368745 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-autosocket.xml @@ -0,0 +1,39 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes'/> + <video> + <model type='cirrus' vram='16384' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index f766f4dc521e..62f8f96c2274 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -263,6 +263,7 @@ mymain(void) { int ret = 0; struct testInfo info; + virQEMUDriverConfigPtr cfg = NULL; if (qemuTestDriverInit(&driver) < 0) return EXIT_FAILURE; @@ -774,6 +775,12 @@ mymain(void) DO_TEST("virtio-input"); DO_TEST("virtio-input-passthrough"); + cfg = virQEMUDriverGetConfig(&driver); + cfg->vncAutoUnixSocket = true; + DO_TEST_FULL("graphics-vnc-autosocket", WHEN_INACTIVE, NONE); + cfg->vncAutoUnixSocket = false; + + virObjectUnref(cfg); qemuTestDriverFree(&driver); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; -- 2.8.1

2 3

[libvirt] [PATCH] qemu: Error out if setting vcpu count would lead to invalid config
by Peter Krempa 28 Apr '16

28 Apr '16

When the domain definition describes a machine with NUMA, setting the maximum vCPU count via the API might lead to an invalid config. Add a check that will forbid this until we add more advanced cpu config capabilities. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1327499 --- src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ad5c382..769bdb7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -673,6 +673,7 @@ virNodeDeviceObjUnlock; virDomainNumaCheckABIStability; virDomainNumaEquals; virDomainNumaFree; +virDomainNumaGetCPUCountTotal; virDomainNumaGetMaxCPUID; virDomainNumaGetMemorySize; virDomainNumaGetNodeCount; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 542d13c..44ec42d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4898,6 +4898,13 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (persistentDef) { if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { + if (virDomainNumaGetCPUCountTotal(persistentDef->numa) > nvcpus) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Number of CPUs in <numa> exceeds the desired " + "maximum vcpu count")); + goto endjob; + } + if (virDomainDefSetVcpusMax(persistentDef, nvcpus) < 0) goto endjob; } else { -- 2.8.1

2 2

[libvirt] [PATCH v2] qemu: conf: Set default logging approach in virQEMUDriverConfigNew
by Peter Krempa 28 Apr '16

28 Apr '16

Instead of setting the default qemu stdio logging approach in virQEMUDriverConfigLoadFile set it in virQEMUDriverConfigNew so that it's properly set even when the config is not present. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1325075 --- src/qemu/qemu_conf.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 77ef4fe..5ed5776 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -324,6 +324,7 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) cfg->seccompSandbox = -1; cfg->logTimestamp = true; + cfg->stdioLogD = true; #ifdef DEFAULT_LOADER_NVRAM if (virQEMUDriverConfigLoaderNVRAMParse(cfg, DEFAULT_LOADER_NVRAM) < 0) @@ -812,8 +813,6 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, goto cleanup; } VIR_FREE(stdioHandler); - } else { - cfg->stdioLogD = true; } GET_VALUE_ULONG("max_queued", cfg->maxQueuedJobs); -- 2.8.1

2 2

[libvirt] [PATCH] trivial: zh_CN: fix the wrong translation
by Wanlong Gao 28 Apr '16

28 Apr '16

From: Allen Gao <wanlong.gao(a)easystack.cn> s/数值/设置/ Signed-off-by: Wanlong Gao <wanlong.gao(a)easystack.cn> --- po/zh_CN.po | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/po/zh_CN.po b/po/zh_CN.po index 05d860f..54b75ce 100644 --- a/po/zh_CN.po +++ b/po/zh_CN.po @@ -34600,7 +34600,7 @@ msgstr "无法解析整数参数" #: tools/virsh-domain.c:1473 msgid "Get or set blkio parameters" -msgstr "获取或者数值 blkio 参数" +msgstr "获取或者设置 blkio 参数" #: tools/virsh-domain.c:1476 msgid "" @@ -34609,7 +34609,7 @@ msgid "" "\n" " virsh # blkiotune <domain>" msgstr "" -"为虚拟机域获取或者数值当前 blkio 参数。\n" +"为虚拟机域获取或者设置当前 blkio 参数。\n" " 请使用以下命令获取 blkio 参数： \n" "\n" " virsh # blkiotune <domain>" -- 2.5.0

2 1

[libvirt] RFC: extending domcapabilities XML
by Cole Robinson 27 Apr '16

27 Apr '16

Hi all, There's a couple qemu bits I want to expose in domcapabilities XML, but there isn't an existing pattern to follow so I'd like to get some feedback first. Hopefully it can serve as a template for future domcapabilities extensions 1) USB controller models Not strictly important for my needs, but this is an incremental step. Do we do some kind of nested XML like <controller supported='yes'> <type name='usb'/> <enum name='model'> <value>nec-xhci</value> ... </enum> </type> </controller> Or come up with a unique device-level enum name to track this, like we do for hostdev subsysType: <controller supported='yes'> <enum name='usbModel'> <value>nec-xhci</value> ... </enum> </controller> 2) new usb ports= attribute This is for the patches I just sent. There's two questions here: how does XML look like for a non-enum? Maybe: <controller supported='yes'> <parameter name='ports' supported='yes'/> </controller> (or maybe <option .../>) However, ports is only supported for type=usb model=nec-xhci. Do we represent that? If so what does the XML look like? Do we go crazy nested like: <controller supported='yes'> <parameter name='type' value='usb'> <parameter name='model' value='nec-xhci'/> <parameter name='ports' supported='yes'/> </parameter> </parameter> </controller> But that's kind of intense. It gives the illusion that we can write a parser to automatically handle additions for introspecting what libvirt supports, but in practice I don't know if that's ever going to work. Instead I think it's better to invent new top level strings to represent these types of cases, rather than try to encode an infinite level of dependency in the XML hierarchy. <controller supported='yes'> <capability name='usbXHCIPorts' supported='yes'/> </controller> The question then is if the <capability> element is the proper name. Could be <feature> too but that's quite overloaded in libvirt XML. 3) spice gl whether spice gl is supported or not is a factor of libvirt version, qemu version, and spice version that qemu was compiled against, which is a lot of pieces to track. So exposing it over domcapabilities will be nice. Something like: <graphics supported='yes'/> <enum name='spiceGL'> <value>off</value> <value>on</value> </enum> </graphics> Similarly we want to know that virtio-vga has virgl support via <acceleration accel3d='yes'/> <video supported='yes'/> <enum name='virtioVGAAccel3D'> <value>yes</value> <value>no</value> </enum> </video> Comments welcome. Thanks, Cole

1 0

[libvirt] [PATCH] rpc: Don't leak fd via CreateXMLWithFiles
by Cole Robinson 27 Apr '16

27 Apr '16

From: Ben Gray <ben.r.gray(a)gmail.com> FD passing APIs like CreateXMLWithFiles or OpenGraphicsFD will leak file descriptors. The user passes in an fd, which is dup()'d in virNetClientProgramCall. The new fd is what is transfered to the server virNetClientIOWriteMessage. Once all the fds have been written though, the parent msg->fds list is immediately free'd, so the individual fds are never closed. This closes each FD as its send to the server, so all fds have been closed by the time msg->fds is free'd. https://bugzilla.redhat.com/show_bug.cgi?id=1159766 Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- Ben's patch, my comments src/rpc/virnetclient.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 781e74c..d8ed15b 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -1177,20 +1177,21 @@ virNetClientIOWriteMessage(virNetClientPtr client, if (thecall->msg->bufferOffset == thecall->msg->bufferLength) { size_t i; for (i = thecall->msg->donefds; i < thecall->msg->nfds; i++) { int rv; if ((rv = virNetSocketSendFD(client->sock, thecall->msg->fds[i])) < 0) return -1; if (rv == 0) /* Blocking */ return 0; thecall->msg->donefds++; + VIR_FORCE_CLOSE(thecall->msg->fds[i]); } thecall->msg->donefds = 0; thecall->msg->bufferOffset = thecall->msg->bufferLength = 0; VIR_FREE(thecall->msg->fds); VIR_FREE(thecall->msg->buffer); if (thecall->expectReply) thecall->mode = VIR_NET_CLIENT_MODE_WAIT_RX; else thecall->mode = VIR_NET_CLIENT_MODE_COMPLETE; } -- 2.7.3

3 5

[libvirt] [PATCH 0/3] conf: reject '/' in object names
by Cole Robinson 27 Apr '16

27 Apr '16

An embedded '/' in object names doesn't really work for any of our stateful drivers, so let's explicitly reject it. https://bugzilla.redhat.com/show_bug.cgi?id=639923 https://bugzilla.redhat.com/show_bug.cgi?id=787604 Cole Robinson (3): conf: domain: reject name containing '/' conf: network: reject name containing '/' conf: storage: pool: reject name containing '/' src/conf/domain_conf.c | 8 ++++++++ src/conf/domain_conf.h | 1 + src/conf/network_conf.c | 6 ++++++ src/conf/storage_conf.c | 6 ++++++ src/openvz/openvz_driver.c | 5 +++-- src/phyp/phyp_driver.c | 1 + src/vbox/vbox_common.c | 1 + src/vmx/vmx.c | 3 ++- src/xenapi/xenapi_driver.c | 1 + tests/genericxml2xmlindata/generic-name-slash-fail.xml | 17 +++++++++++++++++ tests/genericxml2xmltest.c | 3 +++ 11 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 tests/genericxml2xmlindata/generic-name-slash-fail.xml -- 2.7.4

2 6

[libvirt] [PATCH] conf: format runtime DAC seclabel, unless MIGRATABLE
by Cole Robinson 27 Apr '16

27 Apr '16

We historically format runtime seclabel selinux/apparmor values, however we skip formatting runtime DAC values. This was added in commit 990e46c4542349f838e001d30638872576c389e9 Author: Marcelo Cerri <mhcerri(a)linux.vnet.ibm.com> Date: Fri Aug 31 13:40:41 2012 +0200 conf: Avoid formatting auto-generated DAC labels to maintain migration compatibility with libvirt < 0.10.0. However the formatting was skipped unconditionally. Instead only skip formatting in the VIR_DOMAIN_DEF_FORMAT_MIGRATABLE case. https://bugzilla.redhat.com/show_bug.cgi?id=1215833 --- src/conf/domain_conf.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index db567f5..0557912 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18741,7 +18741,8 @@ virDomainEventActionDefFormat(virBufferPtr buf, static void virSecurityLabelDefFormat(virBufferPtr buf, - virSecurityLabelDefPtr def) + virSecurityLabelDefPtr def, + unsigned int flags) { const char *sectype = virDomainSeclabelTypeToString(def->type); @@ -18751,11 +18752,13 @@ virSecurityLabelDefFormat(virBufferPtr buf, if (def->type == VIR_DOMAIN_SECLABEL_DEFAULT) return; - /* To avoid backward compatibility issues, suppress DAC and 'none' labels - * that are automatically generated. + /* libvirt versions prior to 0.10.0 support just a single seclabel element + * in the XML, and that would typically be filled with type=selinux. + * Don't format it in the MIGRATABLE case, for backwards compatibility */ if ((STREQ_NULLABLE(def->model, "dac") || - STREQ_NULLABLE(def->model, "none")) && def->implicit) + STREQ_NULLABLE(def->model, "none")) && def->implicit && + (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE)) return; virBufferAsprintf(buf, "<seclabel type='%s'", @@ -22886,7 +22889,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, "</devices>\n"); for (n = 0; n < def->nseclabels; n++) - virSecurityLabelDefFormat(buf, def->seclabels[n]); + virSecurityLabelDefFormat(buf, def->seclabels[n], flags); if (def->namespaceData && def->ns.format) { if ((def->ns.format)(buf, def->namespaceData) < 0) -- 2.7.3

2 2

[libvirt] [PATCHv2] conf: log error when incorrect PCI root controller is added to domain
by Laine Stump 27 Apr '16

27 Apr '16

libvirt may automatically add a pci-root or pcie-root controller to a domain, depending on the arch/machinetype, and it hopefully always makes the right decision about which to add (since in all cases these controllers are an implicit part of the virtual machine). But it's always possible that someone will create a config that explicitly supplies the wrong type of PCI controller for the selected machinetype. In the past that would lead to an error later when libvirt was trying to assign addresses to other devices, for example: XML error: PCI bus is not compatible with the device at 0000:00:02.0. Device requires a PCI Express slot, which is not provided by bus 0000:00 (that's the error message that appears if you replace the pcie-root controller in a Q35 domain with a pci-root controller). This patch adds a check at the same place that the implicit controllers are added (to ensure that the same logic is used to check which type of pci root is correct). If a pci controller with index='0' is already present, we verify that it is of the model that we would have otherwise added automatically; if not, an error is logged: The PCI controller with index='0' must be " model='pcie-root' for this machine type, " but model='pci-root' was found instead. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1004602 --- Change from V1 - add test cases per Cole's suggestion (one for 440fx, one for Q35) src/qemu/qemu_domain.c | 47 ++++++++++++++++------ .../qemuxml2argv-440fx-wrong-root.xml | 28 +++++++++++++ .../qemuxml2argv-q35-wrong-root.xml | 27 +++++++++++++ tests/qemuxml2argvtest.c | 9 +++++ 4 files changed, 99 insertions(+), 12 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-440fx-wrong-root.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-wrong-root.xml diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 51d4830..86b7d13 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1437,6 +1437,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, { bool addDefaultUSB = true; int usbModel = -1; /* "default for machinetype" */ + int pciRoot; /* index within def->controllers */ bool addImplicitSATA = false; bool addPCIRoot = false; bool addPCIeRoot = false; @@ -1538,14 +1539,26 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, def, VIR_DOMAIN_CONTROLLER_TYPE_SATA, 0, -1) < 0) goto cleanup; + pciRoot = virDomainControllerFind(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0); + /* NB: any machine that sets addPCIRoot to true must also return * true from the function qemuDomainSupportsPCI(). */ - if (addPCIRoot && - virDomainDefMaybeAddController( - def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0, - VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0) - goto cleanup; + if (addPCIRoot) { + if (pciRoot >= 0) { + if (def->controllers[pciRoot]->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) { + virReportError(VIR_ERR_XML_ERROR, + _("The PCI controller with index='0' must be " + "model='pci-root' for this machine type, " + "but model='%s' was found instead"), + virDomainControllerModelPCITypeToString(def->controllers[pciRoot]->model)); + goto cleanup; + } + } else if (!virDomainDefAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0, + VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)) { + goto cleanup; + } + } /* When a machine has a pcie-root, make sure that there is always * a dmi-to-pci-bridge controller added as bus 1, and a pci-bridge @@ -1555,15 +1568,25 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, * true from the function qemuDomainSupportsPCI(). */ if (addPCIeRoot) { + if (pciRoot >= 0) { + if (def->controllers[pciRoot]->model != VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) { + virReportError(VIR_ERR_XML_ERROR, + _("The PCI controller with index='0' must be " + "model='pcie-root' for this machine type, " + "but model='%s' was found instead"), + virDomainControllerModelPCITypeToString(def->controllers[pciRoot]->model)); + goto cleanup; + } + } else if (!virDomainDefAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0, + VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)) { + goto cleanup; + } if (virDomainDefMaybeAddController( - def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0, - VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) < 0 || - virDomainDefMaybeAddController( - def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 1, - VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE) < 0 || + def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 1, + VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE) < 0 || virDomainDefMaybeAddController( - def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 2, - VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE) < 0) { + def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 2, + VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE) < 0) { goto cleanup; } } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-440fx-wrong-root.xml b/tests/qemuxml2argvdata/qemuxml2argv-440fx-wrong-root.xml new file mode 100644 index 0000000..93fef08 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-440fx-wrong-root.xml @@ -0,0 +1,28 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <title>A description of the test machine.</title> + <description> + A test of qemu's minimal configuration. + This test also tests the description and title elements. + </description> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-kvm</emulator> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pcie-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-wrong-root.xml b/tests/qemuxml2argvdata/qemuxml2argv-q35-wrong-root.xml new file mode 100644 index 0000000..836de52 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-wrong-root.xml @@ -0,0 +1,27 @@ +<domain type='qemu'> + <name>q35-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-kvm</emulator> + <controller type='pci' index='0' model='pci-root'/> + <controller type='pci' index='1' model='dmi-to-pci-bridge'> + <model name='i82801b11-bridge'/> + </controller> + <controller type='pci' index='2' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='56'/> + </controller> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 8842b2f..b43f03c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1536,6 +1536,15 @@ mymain(void) QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL); + DO_TEST_PARSE_ERROR("q35-wrong-root", + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1, + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL); + DO_TEST_PARSE_ERROR("440fx-wrong-root", NONE); + DO_TEST_FAILURE("pcie-root-port-too-many", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, -- 2.5.5

2 2

[libvirt] [PATCH v2] configure: Remove build time checks for (ip|ip6|eb)tables
by Cole Robinson 27 Apr '16

27 Apr '16

And the 'ip' tool. There isn't much benefit to checking this at configure time when we have infrastructure nowadays for looking up binaries in the PATH --- v2: Keep the virFileIsExecutable check configure.ac | 12 ------ src/util/virfirewall.c | 18 +++++---- src/util/virnetdev.c | 6 +-- tests/virfirewalltest.c | 98 ++++++++++++++++++++++++------------------------- 4 files changed, 62 insertions(+), 72 deletions(-) diff --git a/configure.ac b/configure.ac index de5f430..35ae16e 100644 --- a/configure.ac +++ b/configure.ac @@ -694,18 +694,6 @@ if test x"$with_rhel5_api" = x"yes"; then AC_DEFINE([WITH_RHEL5_API], [1], [whether building for the RHEL-5 API]) fi -AC_PATH_PROG([IP_PATH], [ip], /sbin/ip, [/usr/sbin:$PATH]) -AC_DEFINE_UNQUOTED([IP_PATH], "$IP_PATH", [path to ip binary]) - -AC_PATH_PROG([IPTABLES_PATH], [iptables], /sbin/iptables, [/usr/sbin:$PATH]) -AC_DEFINE_UNQUOTED([IPTABLES_PATH], "$IPTABLES_PATH", [path to iptables binary]) - -AC_PATH_PROG([IP6TABLES_PATH], [ip6tables], /sbin/ip6tables, [/usr/sbin:$PATH]) -AC_DEFINE_UNQUOTED([IP6TABLES_PATH], "$IP6TABLES_PATH", [path to ip6tables binary]) - -AC_PATH_PROG([EBTABLES_PATH], [ebtables], /sbin/ebtables, [/usr/sbin:$PATH]) -AC_DEFINE_UNQUOTED([EBTABLES_PATH], "$EBTABLES_PATH", [path to ebtables binary]) - dnl dnl Checks for the OpenVZ driver diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index f26fd86..0c8e3bf 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -47,9 +47,9 @@ typedef virFirewallGroup *virFirewallGroupPtr; VIR_ENUM_DECL(virFirewallLayerCommand) VIR_ENUM_IMPL(virFirewallLayerCommand, VIR_FIREWALL_LAYER_LAST, - EBTABLES_PATH, - IPTABLES_PATH, - IP6TABLES_PATH); + "ebtables", + "iptables", + "ip6tables"); VIR_ENUM_DECL(virFirewallLayerFirewallD) VIR_ENUM_IMPL(virFirewallLayerFirewallD, VIR_FIREWALL_LAYER_LAST, @@ -134,13 +134,13 @@ static void virFirewallCheckUpdateLocking(void) { const char *iptablesArgs[] = { - IPTABLES_PATH, "-w", "-L", "-n", NULL, + "iptables", "-w", "-L", "-n", NULL, }; const char *ip6tablesArgs[] = { - IP6TABLES_PATH, "-w", "-L", "-n", NULL, + "ip6tables", "-w", "-L", "-n", NULL, }; const char *ebtablesArgs[] = { - EBTABLES_PATH, "--concurrent", "-L", NULL, + "ebtables", "--concurrent", "-L", NULL, }; if (lockOverride) return; @@ -182,17 +182,19 @@ virFirewallValidateBackend(virFirewallBackend backend) if (backend == VIR_FIREWALL_BACKEND_DIRECT) { const char *commands[] = { - IPTABLES_PATH, IP6TABLES_PATH, EBTABLES_PATH + "iptables", "ip6tables", "ebtables" }; size_t i; for (i = 0; i < ARRAY_CARDINALITY(commands); i++) { - if (!virFileIsExecutable(commands[i])) { + char *path = virFindFileInPath(commands[i]); + if (!path || !virFileIsExecutable(path)) { virReportSystemError(errno, _("direct firewall backend requested, but %s is not available"), commands[i]); return -1; } + VIR_FREE(path); } VIR_DEBUG("found iptables/ip6tables/ebtables, using direct backend"); } diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index bb17b84..75e45fd 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1469,7 +1469,7 @@ int virNetDevSetIPAddress(const char *ifname, virCommandAddArgList(cmd, "broadcast", bcaststr, NULL); virCommandAddArg(cmd, "alias"); # else - cmd = virCommandNew(IP_PATH); + cmd = virCommandNew("ip"); virCommandAddArgList(cmd, "addr", "add", NULL); virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix); if (peerstr) @@ -1506,7 +1506,7 @@ virNetDevAddRoute(const char *ifname, goto cleanup; if (!(gatewaystr = virSocketAddrFormat(gateway))) goto cleanup; - cmd = virCommandNew(IP_PATH); + cmd = virCommandNew("ip"); virCommandAddArgList(cmd, "route", "add", NULL); virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix); virCommandAddArgList(cmd, "via", gatewaystr, "dev", ifname, @@ -1544,7 +1544,7 @@ int virNetDevClearIPAddress(const char *ifname, virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix); virCommandAddArg(cmd, "-alias"); # else - cmd = virCommandNew(IP_PATH); + cmd = virCommandNew("ip"); virCommandAddArgList(cmd, "addr", "del", NULL); virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix); virCommandAddArgList(cmd, "dev", ifname, NULL); diff --git a/tests/virfirewalltest.c b/tests/virfirewalltest.c index f1f29c6..976e883 100644 --- a/tests/virfirewalltest.c +++ b/tests/virfirewalltest.c @@ -128,11 +128,11 @@ VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block, if (fwBuf) { if (STREQ(type, "ipv4")) - virBufferAddLit(fwBuf, IPTABLES_PATH); + virBufferAddLit(fwBuf, "iptables"); else if (STREQ(type, "ipv4")) - virBufferAddLit(fwBuf, IP6TABLES_PATH); + virBufferAddLit(fwBuf, "ip6tables"); else - virBufferAddLit(fwBuf, EBTABLES_PATH); + virBufferAddLit(fwBuf, "ebtables"); } for (i = 0; i < nargs; i++) { if (fwBuf) { @@ -204,8 +204,8 @@ testFirewallSingleGroup(const void *opaque) int ret = -1; const char *actual = NULL; const char *expected = - IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n" - IPTABLES_PATH " -A INPUT --source-host '!192.168.122.1' --jump REJECT\n"; + "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n" + "iptables -A INPUT --source-host '!192.168.122.1' --jump REJECT\n"; const struct testFirewallData *data = opaque; fwDisabled = data->fwDisabled; @@ -263,8 +263,8 @@ testFirewallRemoveRule(const void *opaque) int ret = -1; const char *actual = NULL; const char *expected = - IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n" - IPTABLES_PATH " -A INPUT --source-host '!192.168.122.1' --jump REJECT\n"; + "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n" + "iptables -A INPUT --source-host '!192.168.122.1' --jump REJECT\n"; const struct testFirewallData *data = opaque; virFirewallRulePtr fwrule; @@ -329,10 +329,10 @@ testFirewallManyGroups(const void *opaque ATTRIBUTE_UNUSED) int ret = -1; const char *actual = NULL; const char *expected = - IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n" - IPTABLES_PATH " -A INPUT --source-host '!192.168.122.1' --jump REJECT\n" - IPTABLES_PATH " -A OUTPUT --source-host 192.168.122.1 --jump ACCEPT\n" - IPTABLES_PATH " -A OUTPUT --jump DROP\n"; + "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n" + "iptables -A INPUT --source-host '!192.168.122.1' --jump REJECT\n" + "iptables -A OUTPUT --source-host 192.168.122.1 --jump ACCEPT\n" + "iptables -A OUTPUT --jump DROP\n"; const struct testFirewallData *data = opaque; fwDisabled = data->fwDisabled; @@ -423,10 +423,10 @@ testFirewallIgnoreFailGroup(const void *opaque ATTRIBUTE_UNUSED) int ret = -1; const char *actual = NULL; const char *expected = - IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n" - IPTABLES_PATH " -A INPUT --source-host 192.168.122.255 --jump REJECT\n" - IPTABLES_PATH " -A OUTPUT --source-host 192.168.122.1 --jump ACCEPT\n" - IPTABLES_PATH " -A OUTPUT --jump DROP\n"; + "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n" + "iptables -A INPUT --source-host 192.168.122.255 --jump REJECT\n" + "iptables -A OUTPUT --source-host 192.168.122.1 --jump ACCEPT\n" + "iptables -A OUTPUT --jump DROP\n"; const struct testFirewallData *data = opaque; fwDisabled = data->fwDisabled; @@ -498,10 +498,10 @@ testFirewallIgnoreFailRule(const void *opaque ATTRIBUTE_UNUSED) int ret = -1; const char *actual = NULL; const char *expected = - IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n" - IPTABLES_PATH " -A INPUT --source-host 192.168.122.255 --jump REJECT\n" - IPTABLES_PATH " -A OUTPUT --source-host 192.168.122.1 --jump ACCEPT\n" - IPTABLES_PATH " -A OUTPUT --jump DROP\n"; + "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n" + "iptables -A INPUT --source-host 192.168.122.255 --jump REJECT\n" + "iptables -A OUTPUT --source-host 192.168.122.1 --jump ACCEPT\n" + "iptables -A OUTPUT --jump DROP\n"; const struct testFirewallData *data = opaque; fwDisabled = data->fwDisabled; @@ -572,8 +572,8 @@ testFirewallNoRollback(const void *opaque ATTRIBUTE_UNUSED) int ret = -1; const char *actual = NULL; const char *expected = - IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n" - IPTABLES_PATH " -A INPUT --source-host 192.168.122.255 --jump REJECT\n"; + "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n" + "iptables -A INPUT --source-host 192.168.122.255 --jump REJECT\n"; const struct testFirewallData *data = opaque; fwDisabled = data->fwDisabled; @@ -642,11 +642,11 @@ testFirewallSingleRollback(const void *opaque ATTRIBUTE_UNUSED) int ret = -1; const char *actual = NULL; const char *expected = - IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n" - IPTABLES_PATH " -A INPUT --source-host 192.168.122.255 --jump REJECT\n" - IPTABLES_PATH " -D INPUT --source-host 192.168.122.1 --jump ACCEPT\n" - IPTABLES_PATH " -D INPUT --source-host 192.168.122.255 --jump REJECT\n" - IPTABLES_PATH " -D INPUT --source-host '!192.168.122.1' --jump REJECT\n"; + "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n" + "iptables -A INPUT --source-host 192.168.122.255 --jump REJECT\n" + "iptables -D INPUT --source-host 192.168.122.1 --jump ACCEPT\n" + "iptables -D INPUT --source-host 192.168.122.255 --jump REJECT\n" + "iptables -D INPUT --source-host '!192.168.122.1' --jump REJECT\n"; const struct testFirewallData *data = opaque; fwDisabled = data->fwDisabled; @@ -732,10 +732,10 @@ testFirewallManyRollback(const void *opaque ATTRIBUTE_UNUSED) int ret = -1; const char *actual = NULL; const char *expected = - IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n" - IPTABLES_PATH " -A INPUT --source-host 192.168.122.255 --jump REJECT\n" - IPTABLES_PATH " -D INPUT --source-host 192.168.122.255 --jump REJECT\n" - IPTABLES_PATH " -D INPUT --source-host '!192.168.122.1' --jump REJECT\n"; + "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n" + "iptables -A INPUT --source-host 192.168.122.255 --jump REJECT\n" + "iptables -D INPUT --source-host 192.168.122.255 --jump REJECT\n" + "iptables -D INPUT --source-host '!192.168.122.1' --jump REJECT\n"; const struct testFirewallData *data = opaque; fwDisabled = data->fwDisabled; @@ -825,14 +825,14 @@ testFirewallChainedRollback(const void *opaque ATTRIBUTE_UNUSED) int ret = -1; const char *actual = NULL; const char *expected = - IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n" - IPTABLES_PATH " -A INPUT --source-host 192.168.122.127 --jump REJECT\n" - IPTABLES_PATH " -A INPUT --source-host '!192.168.122.1' --jump REJECT\n" - IPTABLES_PATH " -A INPUT --source-host 192.168.122.255 --jump REJECT\n" - IPTABLES_PATH " -D INPUT --source-host 192.168.122.127 --jump REJECT\n" - IPTABLES_PATH " -D INPUT --source-host '!192.168.122.1' --jump REJECT\n" - IPTABLES_PATH " -D INPUT --source-host 192.168.122.255 --jump REJECT\n" - IPTABLES_PATH " -D INPUT --source-host '!192.168.122.1' --jump REJECT\n"; + "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n" + "iptables -A INPUT --source-host 192.168.122.127 --jump REJECT\n" + "iptables -A INPUT --source-host '!192.168.122.1' --jump REJECT\n" + "iptables -A INPUT --source-host 192.168.122.255 --jump REJECT\n" + "iptables -D INPUT --source-host 192.168.122.127 --jump REJECT\n" + "iptables -D INPUT --source-host '!192.168.122.1' --jump REJECT\n" + "iptables -D INPUT --source-host 192.168.122.255 --jump REJECT\n" + "iptables -D INPUT --source-host '!192.168.122.1' --jump REJECT\n"; const struct testFirewallData *data = opaque; fwDisabled = data->fwDisabled; @@ -976,11 +976,11 @@ testFirewallQueryHook(const char *const*args, int *status, void *opaque ATTRIBUTE_UNUSED) { - if (STREQ(args[0], IPTABLES_PATH) && + if (STREQ(args[0], "iptables") && STREQ(args[1], "-L")) { if (VIR_STRDUP(*output, TEST_FILTER_TABLE_LIST) < 0) *status = 127; - } else if (STREQ(args[0], IPTABLES_PATH) && + } else if (STREQ(args[0], "iptables") && STREQ(args[1], "-t") && STREQ(args[2], "nat") && STREQ(args[3], "-L")) { @@ -1026,15 +1026,15 @@ testFirewallQuery(const void *opaque ATTRIBUTE_UNUSED) int ret = -1; const char *actual = NULL; const char *expected = - IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n" - IPTABLES_PATH " -A INPUT --source-host 192.168.122.127 --jump REJECT\n" - IPTABLES_PATH " -L\n" - IPTABLES_PATH " -t nat -L\n" - IPTABLES_PATH " -A INPUT --source-host 192.168.122.130 --jump REJECT\n" - IPTABLES_PATH " -A INPUT --source-host '!192.168.122.129' --jump REJECT\n" - IPTABLES_PATH " -A INPUT --source-host '!192.168.122.129' --jump REJECT\n" - IPTABLES_PATH " -A INPUT --source-host 192.168.122.128 --jump REJECT\n" - IPTABLES_PATH " -A INPUT --source-host '!192.168.122.1' --jump REJECT\n"; + "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n" + "iptables -A INPUT --source-host 192.168.122.127 --jump REJECT\n" + "iptables -L\n" + "iptables -t nat -L\n" + "iptables -A INPUT --source-host 192.168.122.130 --jump REJECT\n" + "iptables -A INPUT --source-host '!192.168.122.129' --jump REJECT\n" + "iptables -A INPUT --source-host '!192.168.122.129' --jump REJECT\n" + "iptables -A INPUT --source-host 192.168.122.128 --jump REJECT\n" + "iptables -A INPUT --source-host '!192.168.122.1' --jump REJECT\n"; const struct testFirewallData *data = opaque; expectedLineNum = 0; -- 2.7.3

2 2

[libvirt] [PATCH] qemu: conf: Run through the parser even without config file
by Peter Krempa 27 Apr '16

27 Apr '16

The parser sets a few defaults if the are not present in the config. We should run that even when the file is missing. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1325075 --- src/qemu/qemu_conf.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 77ef4fe..0bd9f06 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -477,12 +477,11 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, */ if (access(filename, R_OK) == -1) { VIR_INFO("Could not read qemu config file %s", filename); - return 0; + } else { + if (!(conf = virConfReadFile(filename, 0))) + goto cleanup; } - if (!(conf = virConfReadFile(filename, 0))) - goto cleanup; - #define CHECK_TYPE(name, typ) \ if (p && p->type != (typ)) { \ virReportError(VIR_ERR_INTERNAL_ERROR, \ -- 2.8.1

2 1

[libvirt] [PATCH 0/2] Fix migrations with automatic vnc sockets
by Martin Kletzander 27 Apr '16

27 Apr '16

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1326270 Martin Kletzander (2): qemu: Unref cfg in qemuDomainDefPostParse qemu: Regenerate VNC socket paths src/qemu/qemu_domain.c | 22 +++++++++++- .../qemuxml2argv-graphics-vnc-autosocket.args | 22 ++++++++++++ .../qemuxml2argv-graphics-vnc-autosocket.xml | 34 +++++++++++++++++++ .../qemuxml2xmlout-graphics-vnc-autosocket.xml | 39 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 7 ++++ 5 files changed, 123 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-autosocket.xml -- 2.8.1

2 9

[libvirt] [PATCH 0/2] qemu: Be able to auto dump core of a domain with long name
by Martin Kletzander 27 Apr '16

27 Apr '16

Yet another fix for https://bugzilla.redhat.com/show_bug.cgi?id=1289363 Martin Kletzander (2): Unify domain name shortening Shorten domain name for automatic coredump src/conf/domain_conf.c | 20 ++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c | 24 ++++++++---------------- src/qemu/qemu_driver.c | 4 +++- 5 files changed, 34 insertions(+), 17 deletions(-) -- 2.8.1

2 4

[libvirt] interface "peer address" patches are broken
by Laine Stump 27 Apr '16

27 Apr '16

I haven't had the time to fix it yet, but wanted to point out that the patches that are supposed to support setting the peer address of tap/macvlan devices do not work. The problem is that the peer attribute isn't being included when the XML for a a domain interface is being formatted. I also sent the following message last week as a followup to the patch series; possibly it wasn't noticed for that reason:. I'll make a patch to fix the formatting when I can so that I can test my theory about address vs. peer on lxc vs qemu. But if it's fixed by someone else in the meantime, all the better :-) On 04/20/2016 12:36 PM, Laine Stump wrote: > Something I've found myself worrying about lately while driving in the > car or nodding off to sleep - are the "address" and "peer" attributes > effectively used in the same way for all network connection types and > both hypervisors? I think the answer may be "no", and if so we need to > fix that before they go out in a release. > > In particular, when an lxc domain's interface has: > > <ip address='192.168.128.1'/> > > That is the IP address seen by the guest, not the host. So I would > assume that if an LXC domain had: > > > <ip address='192.168.128.1' peer='192.168.128.2'/> > > that 192.168.128.1 would still be the IP address see by the guest, and > 192.168.128.2 would be the IP address on the host side; and it should > be the same for qemu. > > From what I can see of the code, though, on a qemu domain, the IP > address is set for the tap device's own IP, meaning that it would show > up on the *host* side, while the peer address would be what the host > expects to be at the other end of the tap device (i.e. the guest > side), so the two attributes are used for the *opposite* end of the > PTP link in lxc vs. qemu. > > I think that, instead, the "address" attribute should *always* be the > IP address that is seen/used by the guest, and the "peer" attribute > should be the IP address that is seen/used by the host. (perhaps > "peer" could be replaced with some other name, like "host" or > "hostAddress" to avoid confusion? (don't like either of those > alternatives, but I don't really like peer either)). > > Aside from that, I can see that these patches have been pushed in the > code that I'm running, and I've been trying to add "peer='blah'" to > interface IP addresses on my test machine, but it's just removed from > the config. Have you tested what got pushed? Has something gone wrong? > > Since there hasn't been a release with these patches included yet, > there is still time to fix it at least to be consistent (assuming that > my suspicions are correct; I've been unable to test it myself for the > reason above).

2 4

[libvirt] [PATCH 0/9] Slightly rework our streams
by Michal Privoznik 26 Apr '16

26 Apr '16

This is not the big patch set that enables sparse streams. Not just yet. I'm merely sending first few patches that prepare the environment for that. These can, however, go in independent of sparse streams. Michal Privoznik (9): daemonClientStream: Use unsigned int to store stream @serial daemon stream: Prefer bool over unsigned int var:1 daemon stream: Convert @tx in daemonClientStream to bool daemon stream: Remove useless empty lines from header file virNetClientCallDispatchStream: Update comment daemonStreamHandleRead: Rework to follow our coding pattern Revert "rpc: Fix slow volume download (virsh vol-download)" virnetclientstream: Process stream messages later virStream{Recv,Send}All: Increase client buffer daemon/stream.c | 116 +++++++++++++++---------------- daemon/stream.h | 2 - src/libvirt-stream.c | 5 +- src/rpc/virnetclient.c | 6 +- src/rpc/virnetclientstream.c | 158 +++++++++++++++--------------------------- src/rpc/virnetserverprogram.c | 12 ++-- src/rpc/virnetserverprogram.h | 4 +- 7 files changed, 129 insertions(+), 174 deletions(-) -- 2.7.3

2 18

[libvirt] [RFC] live migration of VMs with internal snapshots
by Maxim Nestratov 26 Apr '16

26 Apr '16

Hi, As far as I understand, currently there is no way to live migrate qemu VMs that have internal snapshots, because live migration works via qemu drive mirroring, which in turns mirrors only shallow block layer, effectively losing existing embedded snapshots. The problem could be fixed if we created an external delta for all disks with internal snapshots, live migrate such VMs and then auto merge them back into original images. That said, I would like to know very much your opinion on the matter and would also like to know if this approach is affordable. If so, I or one of my colleagues will send a pathset to fix this, otherwise what solution for this problem you will recommend? Thanks, Maxim

5 7

[libvirt] [PATCH] conf: Drop restrictions on rng backend path
by Cole Robinson 26 Apr '16

26 Apr '16

Currently we only allow /dev/random and /dev/hwrng as host input for <rng><backend model='random'/> device. This was added after various upstream discussions in commit 4932ef45 However this restriction has generated quite a few complaints over the years, so a new discussion was initiated: http://www.redhat.com/archives/libvir-list/2016-April/msg00987.html Several people suggested removing the restriction, and nobody really spoke up to defend it. So this patch drops the path restriction entirely https://bugzilla.redhat.com/show_bug.cgi?id=1074464 --- docs/formatdomain.html.in | 9 ++++----- docs/schemas/domaincommon.rng | 3 +-- src/conf/domain_conf.c | 8 -------- tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-rng-random.xml | 2 +- 6 files changed, 8 insertions(+), 18 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 895114b..132b261 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6231,8 +6231,7 @@ qemu-kvm -net nic,model=? /dev/null <code>model</code> attribute. Supported source models are: </p> <ul> - <li>'random' — /dev/random (default) or /dev/hwrng - device as source (for now, no other sources are permitted)</li> + <li>'random' — a character device backend</li> <li>'egd' — a EGD protocol backend</li> </ul> </dd> @@ -6240,9 +6239,9 @@ qemu-kvm -net nic,model=? /dev/null <dd> <p> This backend type expects a non-blocking character device as input. - The only accepted paths are /dev/random and /dev/hwrng. The file - name is specified as contents of the <code>backend</code> element. - When no file name is specified the hypervisor default is used. + The file name is specified as contents of the + <code>backend</code> element. When no file name is specified the + this defaults to /dev/random. </p> </dd> <dt><code>backend model='egd'</code></dt> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3605afe..c91a3a9 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4691,8 +4691,7 @@ <value>random</value> </attribute> <choice> - <value>/dev/random</value> - <value>/dev/hwrng</value> + <ref name='absFilePath'/> <empty/> </choice> </group> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 28248c8..f9cd69c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11512,14 +11512,6 @@ virDomainRNGDefParseXML(xmlNodePtr node, switch ((virDomainRNGBackend) def->backend) { case VIR_DOMAIN_RNG_BACKEND_RANDOM: def->source.file = virXPathString("string(./backend)", ctxt); - if (def->source.file && - STRNEQ(def->source.file, "/dev/random") && - STRNEQ(def->source.file, "/dev/hwrng")) { - virReportError(VIR_ERR_XML_ERROR, - _("file '%s' is not a supported random source"), - def->source.file); - goto error; - } break; case VIR_DOMAIN_RNG_BACKEND_EGD: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args index ba97c1e..5a4d47b 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args @@ -18,6 +18,6 @@ QEMU_AUDIO_DRV=none \ -boot c \ -usb \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \ --object rng-random,id=objrng0,filename=/dev/hwrng \ +-object rng-random,id=objrng0,filename=/dev/urandom \ -device virtio-rng-pci,rng=objrng0,id=rng0,max-bytes=123,period=1234,bus=pci.0,\ addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml index 71bd21a..a6e91ff 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml @@ -21,7 +21,7 @@ <memballoon model='virtio'/> <rng model='virtio'> <rate bytes='123' period='1234'/> - <backend model='random'>/dev/hwrng</backend> + <backend model='random'>/dev/urandom</backend> </rng> </devices> </domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-rng-random.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-rng-random.xml index e8a0478..0bdefde 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-rng-random.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-rng-random.xml @@ -25,7 +25,7 @@ </memballoon> <rng model='virtio'> <rate bytes='123' period='1234'/> - <backend model='random'>/dev/hwrng</backend> + <backend model='random'>/dev/urandom</backend> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </rng> </devices> -- 2.7.3

5 6

[libvirt] [PATCH] spec: Use proper indentation
by Andrea Bolognani 26 Apr '16

26 Apr '16

Commit 1d4400082a02 added some code that was not indented properly and broke syntax-check. Fix it. --- Pushed as trivial. libvirt.spec.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 4eff5ae..970ac32 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1851,11 +1851,11 @@ if test $1 -eq 1 && test ! -f %{_sysconfdir}/libvirt/qemu/networks/default.xml ; ln -s ../default.xml %{_sysconfdir}/libvirt/qemu/networks/autostart/default.xml # Make sure libvirt picks up the new network defininiton - %if %{with_systemd} + %if %{with_systemd} /bin/systemctl try-restart libvirtd.service >/dev/null 2>&1 ||: - %else + %else /sbin/service libvirtd condrestart > /dev/null 2>&1 || : - %endif + %endif fi %endif -- 2.5.5

2 1

[libvirt] [PATCH 00/13] qemu: Kill some very ugly legacy code
by Peter Krempa 26 Apr '16

26 Apr '16

Noticed while doing a depth-first search for a different refactor. Peter Krempa (13): qemu: remove default case from few typecasted enums qemu: hotplug: Assume QEMU_CAPS_DEVICE in qemuDomainAttachUSBMassStorageDevice qemu: hotplug: Assume QEMU_CAPS_DEVICE in qemuDomainAttachHostUSBDevice qemu: monitor: Kill legacy USB monitor code qemu: hotplug: Assume QEMU_CAPS_DEVICE in qemuDomainAttachVirtioDiskDevice qemu: hotplug: Assume QEMU_CAPS_DEVICE in qemuDomainAttachNetDevice qemu: hotplug: Assume QEMU_CAPS_DEVICE in qemuDomainAttachHostPCIDevice qemu: hotplug: Assume QEMU_CAPS_DEVICE in qemuDomainDetachVirtioDiskDevice qemu: hotplug: Assume QEMU_CAPS_DEVICE in qemuDomainDetachControllerDevice qemu: hotplug: Assume QEMU_CAPS_DEVICE in qemuDomainDetachHostPCIDevice qemu: hotplug: Assume QEMU_CAPS_DEVICE in qemuDomainDetachNetDevice qemu: hotplug: Assume QEMU_CAPS_DEVICE in qemuDomainAttachControllerDevice qemu: monitor: Kill legacy PCI hotplug code src/qemu/qemu_command.c | 4 +- src/qemu/qemu_hotplug.c | 381 ++++++++++++++++--------------------------- src/qemu/qemu_monitor.c | 130 --------------- src/qemu/qemu_monitor.h | 37 ----- src/qemu/qemu_monitor_json.c | 79 --------- src/qemu/qemu_monitor_json.h | 11 -- src/qemu/qemu_monitor_text.c | 377 ------------------------------------------ src/qemu/qemu_monitor_text.h | 31 ---- 8 files changed, 140 insertions(+), 910 deletions(-) -- 2.8.1

3 15

[libvirt] ideas for custom iptables rules for libvirt networks.
by Laine Stump 26 Apr '16

26 Apr '16

We still periodically get requests to allow custom iptables rules for libvirt virtual networks (or, more commonly, a mode where libvirt simply leaves iptables alone, not adding or removing anything), and it's been a nagging item on my to-do list for a very long time. The problem is that, although the amount of code required to support *any* solution is very small, it's one of those things without a single obvious "only" way to do it. Anyway, I'm going to take one more stab at it. First, some background points: * For <forward mode='nat'> libvirt's iptables rules are essential to the operation of the forwarding, so we shouldn't mess with that. * For [no forward mode], libvirt's iptables rules are a part of what keeps the network isolated from the rest of the network, so we shouldn't mess with that either. * For <forward mode='route'> we currently allow all outgoing and incoming as long as it is to/from the IP address range defined for the network. So we really want something that can be used only for <forward mode='route'> I can see 3 different possibilities: 1) a new forward mode which is just like 'route', but doesn't add any iptables rules. (what to call it though? "filterless-route"? Too long and ugly :-/) 2) a new attribute to <forward> that takes effect only for mode='route'. Maybe call it "filter". We could have "filter='open'" (what it does currently, and will remain the default), "filter='outgoingOnly'", and "filter='none' (the most requested functionality - no iptables rules would be added for the network) 3) add a <filter> subelement to <forward> that allows specifying iptables rules for the network. Perhaps this could instead be a <filterref>, and use nwfilter to specify the rules? (that sounds really cool, and if it worked it would be a nice re-use of the nwfilter driver, but it may have undetermined pitfalls due to nwfilter being designed with guest traffic filtering in mind, would take a lot more work, and wouldn't address the most common request - "Don't mess with iptables! I want to do it myself!". Anyone have an opinion or alternate idea?

4 5

[libvirt] [PATCH] remote: Don't reject remote polkit if client lacks support
by Cole Robinson 26 Apr '16

26 Apr '16

If you compile a client --without-polkit, and connect to a URI that needs polkit auth, the connection will fail with: $ ./tools/virsh --connect qemu+ssh://crobinso@machine/system error: failed to connect to the hypervisor error: authentication failed: unsupported authentication type 2 This is because the client side portion of the polkit handling is compiled out. However, nothing polkit specific is actually required of the client. Fix that error by unconditionally compiling the basic polkit client handling. https://bugzilla.redhat.com/show_bug.cgi?id=635529 --- Granted, if polkit needs to do any interaction at all, and you are connecting to a remote machine, then things are going to fail anyways with a 'missing agent' error. But if you're user is in the 'libvirt' group polkit doesn't need to auth it should all work. src/remote/remote_driver.c | 69 +++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 38 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index da94411..6bed2c5 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -132,11 +132,9 @@ static int remoteAuthenticate(virConnectPtr conn, struct private_data *priv, #if WITH_SASL static int remoteAuthSASL(virConnectPtr conn, struct private_data *priv, virConnectAuthPtr auth, const char *mech); -#endif -#if WITH_POLKIT +#endif /* WITH_SASL */ static int remoteAuthPolkit(virConnectPtr conn, struct private_data *priv, virConnectAuthPtr auth); -#endif /* WITH_POLKIT */ static virDomainPtr get_nonnull_domain(virConnectPtr conn, remote_nonnull_domain domain); static virNetworkPtr get_nonnull_network(virConnectPtr conn, remote_nonnull_network network); @@ -3326,14 +3324,12 @@ remoteAuthenticate(virConnectPtr conn, struct private_data *priv, } #endif -#if WITH_POLKIT case REMOTE_AUTH_POLKIT: if (remoteAuthPolkit(conn, priv, auth) < 0) { VIR_FREE(ret.types.types_val); return -1; } break; -#endif case REMOTE_AUTH_NONE: /* Nothing todo, hurrah ! */ @@ -3904,30 +3900,10 @@ remoteAuthSASL(virConnectPtr conn, struct private_data *priv, #endif /* WITH_SASL */ -#if WITH_POLKIT -# if WITH_POLKIT1 -static int -remoteAuthPolkit(virConnectPtr conn, struct private_data *priv, - virConnectAuthPtr auth ATTRIBUTE_UNUSED) -{ - remote_auth_polkit_ret ret; - VIR_DEBUG("Client initialize PolicyKit-1 authentication"); - - memset(&ret, 0, sizeof(ret)); - if (call(conn, priv, 0, REMOTE_PROC_AUTH_POLKIT, - (xdrproc_t) xdr_void, (char *)NULL, - (xdrproc_t) xdr_remote_auth_polkit_ret, (char *) &ret) != 0) { - return -1; /* virError already set by call */ - } - - VIR_DEBUG("PolicyKit-1 authentication complete"); - return 0; -} -# elif WITH_POLKIT0 -/* Perform the PolicyKit authentication process - */ +#if WITH_POLKIT0 +/* Perform the PolicyKit0 authentication process */ static int -remoteAuthPolkit(virConnectPtr conn, struct private_data *priv, +remoteAuthPolkit0(virConnectPtr conn, struct private_data *priv, virConnectAuthPtr auth) { remote_auth_polkit_ret ret; @@ -3943,14 +3919,8 @@ remoteAuthPolkit(virConnectPtr conn, struct private_data *priv, }; VIR_DEBUG("Client initialize PolicyKit-0 authentication"); - /* Check auth first and if it succeeds we are done. */ - memset(&ret, 0, sizeof(ret)); - if (call(conn, priv, 0, REMOTE_PROC_AUTH_POLKIT, - (xdrproc_t) xdr_void, (char *)NULL, - (xdrproc_t) xdr_remote_auth_polkit_ret, (char *) &ret) == 0) - goto out; - - /* Auth failed. Ask client to obtain it and check again. */ + /* We only make it here if auth already failed + * Ask client to obtain it and check again. */ if (auth && auth->cb) { /* Check if the necessary credential type for PolicyKit is supported */ for (i = 0; i < auth->ncredtype; i++) { @@ -3986,8 +3956,31 @@ remoteAuthPolkit(virConnectPtr conn, struct private_data *priv, VIR_DEBUG("PolicyKit-0 authentication complete"); return 0; } -# endif /* WITH_POLKIT0 */ -#endif /* WITH_POLKIT */ +#endif /* WITH_POLKIT0 */ + +static int +remoteAuthPolkit(virConnectPtr conn, struct private_data *priv, + virConnectAuthPtr auth ATTRIBUTE_UNUSED) +{ + remote_auth_polkit_ret ret; + VIR_DEBUG("Client initialize PolicyKit authentication"); + + memset(&ret, 0, sizeof(ret)); + if (call(conn, priv, 0, REMOTE_PROC_AUTH_POLKIT, + (xdrproc_t) xdr_void, (char *)NULL, + (xdrproc_t) xdr_remote_auth_polkit_ret, (char *) &ret) != 0) { + return -1; /* virError already set by call */ + } + +#if WITH_POLKIT0 + if (remoteAuthPolkit0(conn, priv, auth) < 0) + return -1; +#endif /* WITH_POLKIT0 */ + + VIR_DEBUG("PolicyKit authentication complete"); + return 0; +} + /*----------------------------------------------------------------------*/ static int -- 2.7.3

2 2

[libvirt] [PATCH v3] schema: Allow space character in disk vendor/product
by Cole Robinson 26 Apr '16

26 Apr '16

The hex range already tried to allow for it, but it wasn't using the correct XML hex syntax. Fix it, and test it --- v3: use proper XML hex syntax (thanks danpb) docs/schemas/domaincommon.rng | 6 ++++-- tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.args | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.xml | 4 ++-- tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-disk-vpd.xml | 4 ++-- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3605afe..66f4167 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1165,14 +1165,16 @@ <optional> <element name="vendor"> <data type="string"> - <param name="pattern">[x20-x7E]{0,8}</param> +  + <param name="pattern">[ -~]{0,8}</param> </data> </element> </optional> <optional> <element name="product"> <data type="string"> - <param name="pattern">[x20-x7E]{0,16}</param> +  + <param name="pattern">[ -~]{0,16}</param> </data> </element> </optional> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.args index df118e3..1831ebd 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.args @@ -24,6 +24,6 @@ QEMU_AUDIO_DRV=none \ -device scsi-cd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,\ id=scsi0-0-0-0,vendor=SEAGATE,product=ST3146707LC \ -drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-scsi1-0-0 \ --device scsi-hd,bus=scsi1.0,scsi-id=0,drive=drive-scsi1-0-0,id=scsi1-0-0,\ -vendor=SEAGATE,product=ST3567807GD \ +-device 'scsi-hd,bus=scsi1.0,scsi-id=0,drive=drive-scsi1-0-0,id=scsi1-0-0,\ +vendor=SEA GATE,product=ST67 807GD' \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.xml index 13004c1..b32b746 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.xml @@ -26,8 +26,8 @@ <source dev='/dev/HostVG/QEMUGuest2'/> <target dev='sdb' bus='scsi'/> <readonly/> - <vendor>SEAGATE</vendor> - <product>ST3567807GD</product> + <vendor>SEA GATE</vendor> + <product>ST67 807GD</product> <address type='drive' controller='1' bus='0' target='0' unit='0'/> </disk> <controller type='usb' index='0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-disk-vpd.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-disk-vpd.xml index 42c7bcd..9e1e7af 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-disk-vpd.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-disk-vpd.xml @@ -26,8 +26,8 @@ <source dev='/dev/HostVG/QEMUGuest2'/> <target dev='sdb' bus='scsi'/> <readonly/> - <vendor>SEAGATE</vendor> - <product>ST3567807GD</product> + <vendor>SEA GATE</vendor> + <product>ST67 807GD</product> <address type='drive' controller='1' bus='0' target='0' unit='0'/> </disk> <controller type='usb' index='0'> -- 2.7.4

2 2

[libvirt] [PATCH v2] schema: Allow space character in disk vendor/product
by Cole Robinson 26 Apr '16

26 Apr '16

The hex range already tried to allow for it, but it doesn't work for reasons I can't figure out. Use a plain character range instead, and test it --- v2: send the fully squashed patch docs/schemas/domaincommon.rng | 6 ++++-- tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.args | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.xml | 4 ++-- tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-disk-vpd.xml | 4 ++-- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3605afe..6c80b66 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1165,14 +1165,16 @@ <optional> <element name="vendor"> <data type="string"> - <param name="pattern">[x20-x7E]{0,8}</param> +  + <param name="pattern">[ -~]{0,8}</param> </data> </element> </optional> <optional> <element name="product"> <data type="string"> - <param name="pattern">[x20-x7E]{0,16}</param> +  + <param name="pattern">[ -~]{0,16}</param> </data> </element> </optional> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.args index df118e3..1831ebd 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.args @@ -24,6 +24,6 @@ QEMU_AUDIO_DRV=none \ -device scsi-cd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,\ id=scsi0-0-0-0,vendor=SEAGATE,product=ST3146707LC \ -drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-scsi1-0-0 \ --device scsi-hd,bus=scsi1.0,scsi-id=0,drive=drive-scsi1-0-0,id=scsi1-0-0,\ -vendor=SEAGATE,product=ST3567807GD \ +-device 'scsi-hd,bus=scsi1.0,scsi-id=0,drive=drive-scsi1-0-0,id=scsi1-0-0,\ +vendor=SEA GATE,product=ST67 807GD' \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.xml index 13004c1..b32b746 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.xml @@ -26,8 +26,8 @@ <source dev='/dev/HostVG/QEMUGuest2'/> <target dev='sdb' bus='scsi'/> <readonly/> - <vendor>SEAGATE</vendor> - <product>ST3567807GD</product> + <vendor>SEA GATE</vendor> + <product>ST67 807GD</product> <address type='drive' controller='1' bus='0' target='0' unit='0'/> </disk> <controller type='usb' index='0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-disk-vpd.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-disk-vpd.xml index 42c7bcd..9e1e7af 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-disk-vpd.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-disk-vpd.xml @@ -26,8 +26,8 @@ <source dev='/dev/HostVG/QEMUGuest2'/> <target dev='sdb' bus='scsi'/> <readonly/> - <vendor>SEAGATE</vendor> - <product>ST3567807GD</product> + <vendor>SEA GATE</vendor> + <product>ST67 807GD</product> <address type='drive' controller='1' bus='0' target='0' unit='0'/> </disk> <controller type='usb' index='0'> -- 2.7.4

2 2

[libvirt] [PATCH v2] spec: If installing default network, restart libvirtd
by Cole Robinson 26 Apr '16

26 Apr '16

If libvirt-daemon-config-network is installed while libvirtd is already running, the daemon doesn't notice the network. Users then have to manually restart libvirtd (or reboot) to pick up the network. Instead let's trigger a daemon restart when the package is first installed. Then the default network is available immediately if libvirtd was already running. https://bugzilla.redhat.com/show_bug.cgi?id=867546 --- v2: Send the patch with actual try-restart/condrestart Plain restart is kind of suboptimal since it will drop all open libvirt connections. Ideally we would use reload which is meant for this case, but it's kinda crashy with current git: http://www.redhat.com/archives/libvir-list/2016-April/msg01624.html And in fact daemon restart is what the spec triggers on libvirt-daemon upgrade anyways, so this isn't anything new WRT packaging. libvirt.spec.in | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/libvirt.spec.in b/libvirt.spec.in index 35212da..4eff5ae 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1849,6 +1849,14 @@ if test $1 -eq 1 && test ! -f %{_sysconfdir}/libvirt/qemu/networks/default.xml ; < %{_datadir}/libvirt/networks/default.xml \ > %{_sysconfdir}/libvirt/qemu/networks/default.xml ln -s ../default.xml %{_sysconfdir}/libvirt/qemu/networks/autostart/default.xml + + # Make sure libvirt picks up the new network defininiton + %if %{with_systemd} + /bin/systemctl try-restart libvirtd.service >/dev/null 2>&1 ||: + %else + /sbin/service libvirtd condrestart > /dev/null 2>&1 || : + %endif + fi %endif -- 2.7.4

2 1

[libvirt] [PATCH] spec: If installing default network, restart libvirtd
by Cole Robinson 26 Apr '16

26 Apr '16

If libvirt-daemon-config-network is installed while libvirtd is already running, the daemon doesn't notice the network. Users then have to manually restart libvirtd (or reboot) to pick up the network. Instead let's trigger a daemon restart when the package is first installed. Then the default network is available immediately if libvirtd was already running. https://bugzilla.redhat.com/show_bug.cgi?id=867546 --- Plain restart is kind of suboptimal since it will drop all open libvirt connections. Ideally we would use reload which is meant for this case, but it's kinda crashy with current git: http://www.redhat.com/archives/libvir-list/2016-April/msg01624.html And in fact daemon restart is what the spec triggers on libvirt-daemon upgrade anyways, so this isn't anything new WRT packaging. libvirt.spec.in | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/libvirt.spec.in b/libvirt.spec.in index 35212da..1a04c88 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1849,6 +1849,14 @@ if test $1 -eq 1 && test ! -f %{_sysconfdir}/libvirt/qemu/networks/default.xml ; < %{_datadir}/libvirt/networks/default.xml \ > %{_sysconfdir}/libvirt/qemu/networks/default.xml ln -s ../default.xml %{_sysconfdir}/libvirt/qemu/networks/autostart/default.xml + + # Make sure libvirt picks up the new network defininiton + %if %{with_systemd} + /bin/systemctl reload libvirtd.service >/dev/null 2>&1 ||: + %else + /sbin/service libvirtd reload > /dev/null 2>&1 || : + %endif + fi %endif -- 2.7.3

2 2