November 2013 - Devel - libvirt List Archives

[libvirt] Libvirt, nwfilter, openvswitch
by Jan De Landtsheer 23 Nov '13

23 Nov '13

Hello, I _was_ trying to set up an nwfilter for our networking set-up with VXLAN and openvswitch, where we use VXLAN as carrier for separate networks (unlike OpenStack gre-tunnels). But with OVS, ebtables do not work, and the basic setup of nwfilter rules are based on this premise... or so I understand... Now.. Is there a way to define nwfilter rules _without_ ebtables ? What I would like to do is quite simple (block out dhcp{4,6} services from VM's, and ipv6 router advertisements ) Thanks

5 10

[libvirt] Fwd: Re: [Qemu-devel] [edk2 PATCH] OvmfPkg: split the variable store to a separate file
by Eric Blake 22 Nov '13

22 Nov '13

[Forwarding with Laszlo's permission - if ever you wonder whether your work on libvirt makes a difference, it does!] -------- Original Message -------- Message-ID: <528FCF54.6060203(a)redhat.com> Date: Fri, 22 Nov 2013 22:40:36 +0100 From: Laszlo Ersek <lersek(a)redhat.com> Subject: Re: [Qemu-devel] [edk2 PATCH] OvmfPkg: split the variable store to a separate file On 11/22/13 21:51, Jordan Justen wrote: > On Fri, Nov 22, 2013 at 10:43 AM, Laszlo Ersek <lersek(a)redhat.com> wrote: >> OTOH building all three FDs per default might be confusing for >> end-users. We know for a fact that they usually don't read the README >> (or not thoroughly enough). If we only give them one output file per >> default, that's less potential for confusion. > > If it will just add confusion, then perhaps it is something best left > out of the README. And at that point, since splitting it is so easy, > the value of having it in OVMF is debatable. > >> But I can certainly post a version where all three files are built per >> default, if that's what you prefer (and aren't opposed to the idea in >> general). > > I'm not opposed to it, but I would like to wait to see what QEMU does. OK. Let's see where the qemu patch goes (I'll have to fix the comment typo that Eric found), and if it is accepted (maybe with modifications), we can return to the OVMF patch. > Many of these scenarios were discussed in the past on qemu-devel, but > a single -pflash was the only thing that stuck. But (thanks to you) we now have a flash driver in OVMF that works *in practice*. The discussion about multiple -pflash flags is not academic any longer. (Hm, sorry, that's maybe too strong -- I can't really say if the discussion used to be academic before. But it cannot have been this practical.) We also know that the libvirt developers prefer the split file. Plus: > This has made me just > focus on making the single file pflash work. You also can't deny that > the QEMU command line gets ugly real fast. We're in violent agreement on that -- which emphasizes the need for good libvirt support all the more. Seriously, I refuse to work with the qemu command line day to day. I have an elaborate wrapper script that I insert between qemu and libvirt (specified in the <emulator> libvirt domain XML) that post-processes the arguments composed by libvirt. I need this to test out features that libvirt doesn't yet support. For example, I can set various filenames in the <loader> element -- it specifies the argument to the -bios option. In the script I check the argument against various patterns, and I can turn it into: - -bios "$ARG" - -pflash "$ARG" - -pflash "$ARG" -pflash "$(manipulate "$ARG")" The script handles CSM vs. pure-UEFI for Windows 2008, it can honor upstream vs. RHEL qemu differences, it can turn off the iPXE virtio-net driver. In the single -pflash option case, it does refresh the code part in the VM's copy of OVMF (with dd) from my most recent build. It sets a name-dependent output file for the debug port. Etc. Maintaining this script over months has paid off orders of magnitude better than working with the raw qemu command line would have. (I can compare: on my (occasionally used) AMD desktop that runs Debian I have not bothered to install libvirt, and each time I need to modify the forty line qemu command that I keep in a script file, I cringe. And I can't bring myself to install a second guest.) The convenience/efficiency libvirt gives me is critical. Changing boot order, adding or removing devices, starting & stopping -- these are very fast in virt-manager. Editing the configuration with virsh is nice (and it has some level of automatic verification when you save and exit). Comparing guest configs against each other (using the XMLs) is very convenient. And so on. I depend on these every day, but I need to modify the wrapper script only occasionally. Libvirt still allows me to send custom monitor commands, and I can set it to log all of its *own* commands that it sends to the monitor or the guest agent. I've always wondered how other developers (for example, you :)) can exist without libvirt at all! That is why "keep the qemu command line simple" (== approx. "two -pflash options are inconvenient") is no concern of mine. That ship has sailed. Thanks, Laszlo

1 0

[libvirt] [PATCH] storage: allow interleave in volume XML
by Eric Blake 22 Nov '13

22 Nov '13

The RNG grammar did not allow arbitrary interleaving, which makes it harder than necessary to create a new volume from handwritten XML. (Compare also to commit caf516db for pools). * docs/schemas/storagevol.rng: Support interleaving. * tests/storagevolxml2xmlin/vol-file-backing.xml: Test it. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- reviewer's note: Look after the '-- ' trailer for a smaller version of the patch generated with 'git diff -w'; that version won't apply, but makes it easier to see the real changes rather than getting lost in indentation. docs/schemas/storagevol.rng | 92 ++++++++++++++------------ tests/storagevolxml2xmlin/vol-file-backing.xml | 15 +++-- 2 files changed, 57 insertions(+), 50 deletions(-) diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index ff7a952..e79bc35 100644 --- a/docs/schemas/storagevol.rng +++ b/docs/schemas/storagevol.rng @@ -12,8 +12,8 @@ <define name='vol'> - <interleave> - <element name='volume'> + <element name='volume'> + <interleave> <element name='name'> <ref name='volName'/> </element> @@ -30,8 +30,8 @@ <optional> <ref name='backingStore'/> </optional> - </element> - </interleave> + </interleave> + </element> </define> <define name='sizing'> @@ -52,20 +52,22 @@ <define name='permissions'> <optional> <element name='permissions'> - <element name='mode'> - <ref name='octalMode'/> - </element> - <element name='owner'> - <ref name='unsignedInt'/> - </element> - <element name='group'> - <ref name='unsignedInt'/> - </element> - <optional> - <element name='label'> - <text/> - </element> - </optional> + <interleave> + <element name='mode'> + <ref name='octalMode'/> + </element> + <element name='owner'> + <ref name='unsignedInt'/> + </element> + <element name='group'> + <ref name='unsignedInt'/> + </element> + <optional> + <element name='label'> + <text/> + </element> + </optional> + </interleave> </element> </optional> </define> @@ -107,36 +109,40 @@ <define name='target'> <element name='target'> - <optional> - <element name='path'> - <choice> - <data type='anyURI'/> - <ref name='absFilePath'/> - </choice> - </element> - </optional> - <ref name='format'/> - <ref name='permissions'/> - <ref name='timestamps'/> - <optional> - <ref name='encryption'/> - </optional> - <optional> - <ref name='compat'/> - </optional> - <optional> - <ref name='fileFormatFeatures'/> - </optional> + <interleave> + <optional> + <element name='path'> + <choice> + <data type='anyURI'/> + <ref name='absFilePath'/> + </choice> + </element> + </optional> + <ref name='format'/> + <ref name='permissions'/> + <ref name='timestamps'/> + <optional> + <ref name='encryption'/> + </optional> + <optional> + <ref name='compat'/> + </optional> + <optional> + <ref name='fileFormatFeatures'/> + </optional> + </interleave> </element> </define> <define name='backingStore'> <element name='backingStore'> - <element name='path'> - <ref name='absFilePath'/> - </element> - <ref name='format'/> - <ref name='permissions'/> + <interleave> + <element name='path'> + <ref name='absFilePath'/> + </element> + <ref name='format'/> + <ref name='permissions'/> + </interleave> </element> </define> diff --git a/tests/storagevolxml2xmlin/vol-file-backing.xml b/tests/storagevolxml2xmlin/vol-file-backing.xml index 73e7f28..8610ea4 100644 --- a/tests/storagevolxml2xmlin/vol-file-backing.xml +++ b/tests/storagevolxml2xmlin/vol-file-backing.xml @@ -1,25 +1,26 @@ <volume> - <name>sparse.img</name> +  <key>/var/lib/libvirt/images/sparse.img</key> - <source/> <capacity unit='GB'>10</capacity> - <allocation unit='MiB'>0</allocation> + <source/> <target> - <path>/var/lib/libvirt/images/sparse.img</path> <permissions> - <mode>0</mode> <owner>0744</owner> + <mode>0</mode> <group>0</group> </permissions> + <path>/var/lib/libvirt/images/sparse.img</path> </target> + <allocation unit='MiB'>0</allocation> <backingStore> - <path>/var/lib/virt/images/master.img</path> <format type='vmdk'/> + <path>/var/lib/virt/images/master.img</path> <permissions> <mode>0744</mode> + <label>virt_image_t</label> <owner>1</owner> <group>1</group> - <label>virt_image_t</label> </permissions> </backingStore> + <name>sparse.img</name> </volume> -- 1.8.3.1 As promised above, the docs/schemas change in an easier format: diff --git c/docs/schemas/storagevol.rng w/docs/schemas/storagevol.rng index ff7a952..e79bc35 100644 --- c/docs/schemas/storagevol.rng +++ w/docs/schemas/storagevol.rng @@ -12,8 +12,8 @@ <define name='vol'> - <interleave> <element name='volume'> + <interleave> <element name='name'> <ref name='volName'/> </element> @@ -30,8 +30,8 @@ <optional> <ref name='backingStore'/> </optional> - </element> </interleave> + </element> </define> <define name='sizing'> @@ -52,6 +52,7 @@ <define name='permissions'> <optional> <element name='permissions'> + <interleave> <element name='mode'> <ref name='octalMode'/> </element> @@ -66,6 +67,7 @@ <text/> </element> </optional> + </interleave> </element> </optional> </define> @@ -107,6 +109,7 @@ <define name='target'> <element name='target'> + <interleave> <optional> <element name='path'> <choice> @@ -127,16 +130,19 @@ <optional> <ref name='fileFormatFeatures'/> </optional> + </interleave> </element> </define> <define name='backingStore'> <element name='backingStore'> + <interleave> <element name='path'> <ref name='absFilePath'/> </element> <ref name='format'/> <ref name='permissions'/> + </interleave> </element> </define>

1 1

[libvirt] [PATCH] virsh domxml-from-native to treat SCSI as the bus type for pseries by default
by Shivaprasad G Bhat 22 Nov '13

22 Nov '13

From: Shivaprasad G Bhat <shivaprasadbhat(a)gmail.com> The bus type IDE being enum Zero, the bus type on pseries system appears as IDE for all the -hda/-cdrom and for disk drives with if="none" type. Pseries platform needs this to appear as SCSI instead of IDE. The ide being not supported, the explicit requests for ide devices will return an error. Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 66 +++++++++++++++++++- tests/qemuargv2xmltest.c | 1 .../qemuxml2argv-pseries-disk.args | 5 ++ .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 40 ++++++++++++ 4 files changed, 108 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8dc7e43..21b5108 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10032,6 +10032,7 @@ error: static virDomainDiskDefPtr qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, const char *val, + virDomainDefPtr dom, int nvirtiodisk, bool old_style_ceph_args) { @@ -10055,7 +10056,11 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, if (VIR_ALLOC(def) < 0) goto cleanup; - def->bus = VIR_DOMAIN_DISK_BUS_IDE; + if (((dom->os.arch == VIR_ARCH_PPC64) && + dom->os.machine && STREQ(dom->os.machine, "pseries"))) + def->bus = VIR_DOMAIN_DISK_BUS_SCSI; + else + def->bus = VIR_DOMAIN_DISK_BUS_IDE; def->device = VIR_DOMAIN_DISK_DEVICE_DISK; def->type = VIR_DOMAIN_DISK_TYPE_FILE; @@ -10140,9 +10145,15 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, def->type = VIR_DOMAIN_DISK_TYPE_FILE; } } else if (STREQ(keywords[i], "if")) { - if (STREQ(values[i], "ide")) + if (STREQ(values[i], "ide")) { def->bus = VIR_DOMAIN_DISK_BUS_IDE; - else if (STREQ(values[i], "scsi")) + if (((dom->os.arch == VIR_ARCH_PPC64) && + dom->os.machine && STREQ(dom->os.machine, "pseries"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("pseries systems do not support ide devices '%s'"), val); + goto error; + } + } else if (STREQ(values[i], "scsi")) def->bus = VIR_DOMAIN_DISK_BUS_SCSI; else if (STREQ(values[i], "virtio")) def->bus = VIR_DOMAIN_DISK_BUS_VIRTIO; @@ -11368,6 +11379,9 @@ qemuParseCommandLine(virCapsPtr qemuCaps, disk->type = VIR_DOMAIN_DISK_TYPE_FILE; if (STREQ(arg, "-cdrom")) { disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; + if (((def->os.arch == VIR_ARCH_PPC64) && + def->os.machine && STREQ(def->os.machine, "pseries"))) + disk->bus = VIR_DOMAIN_DISK_BUS_SCSI; if (VIR_STRDUP(disk->dst, "hdc") < 0) goto error; disk->readonly = true; @@ -11381,6 +11395,9 @@ qemuParseCommandLine(virCapsPtr qemuCaps, disk->bus = VIR_DOMAIN_DISK_BUS_IDE; else disk->bus = VIR_DOMAIN_DISK_BUS_SCSI; + if (((def->os.arch == VIR_ARCH_PPC64) && + def->os.machine && STREQ(def->os.machine, "pseries"))) + disk->bus = VIR_DOMAIN_DISK_BUS_SCSI; } if (VIR_STRDUP(disk->dst, arg + 1) < 0) goto error; @@ -11672,7 +11689,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps, } } else if (STREQ(arg, "-drive")) { WANT_VALUE(); - if (!(disk = qemuParseCommandLineDisk(xmlopt, val, + if (!(disk = qemuParseCommandLineDisk(xmlopt, val, def, nvirtiodisk, ceph_args != NULL))) goto error; @@ -11858,6 +11875,47 @@ qemuParseCommandLine(virCapsPtr qemuCaps, } } else if (STREQ(arg, "-S")) { /* ignore, always added by libvirt */ + } else if (STREQ(arg, "-device")) { + /* something we can't fully parse. Check if supported and + * add it to the qemu namespace + * cmdline/environment advanced options and hope for the best + */ + bool unsupported = false; + WANT_VALUE() + + if ((def->os.arch == VIR_ARCH_PPC64) && + def->os.machine && STREQ(def->os.machine, "pseries")) { + int nkws; + char **kws; + char **vals; + size_t j; + + if (!qemuParseKeywords(val, &kws, &vals, &nkws, 1)) { + for (j = 0; j < nkws; j++) { + if (STREQLEN(kws[j], "ide", 3) || + (STREQ(kws[j], "if") && vals[j] && + STREQ(vals[j], "ide"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("pseries systems do not support ide devices '%s'"), val); + unsupported = true; + break; + } + } + for (j = 0; j < nkws; j++) { + VIR_FREE(kws[j]); + VIR_FREE(vals[j]); + } + VIR_FREE(kws); + VIR_FREE(vals); + } + } + + if (unsupported || VIR_REALLOC_N(cmd->args, cmd->num_args+2) < 0) + goto error; + if (VIR_STRDUP(cmd->args[cmd->num_args++], arg) < 0) + goto error; + if (VIR_STRDUP(cmd->args[cmd->num_args++], val) < 0) + goto error; } else { /* something we can't yet parse. Add it to the qemu namespace * cmdline/environment advanced options and hope for the best diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 6dd8bb0..0bf4c37 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -249,6 +249,7 @@ mymain(void) DO_TEST("hyperv"); DO_TEST("pseries-nvram"); + DO_TEST("pseries-disk"); DO_TEST("nosharepages"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.args new file mode 100644 index 0000000..5fc0938 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.args @@ -0,0 +1,5 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-ppc64 \ +-S -M pseries -m 512 -smp 1 \ +-no-acpi -boot c -usb \ +-boot c -hda /dev/HostVG/QEMUGuest1 -cdrom /root/boot.iso diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml new file mode 100644 index 0000000..dbbd6aa --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml @@ -0,0 +1,40 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>87eedafe-eedc-4336-8130-ed9fe5dc90c8</uuid> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>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-system-ppc64</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='file' device='cdrom'> + <driver name='qemu' type='raw'/> + <source file='/root/boot.iso'/> + <target dev='hdc' bus='scsi'/> + <readonly/> + <address type='drive' controller='0' bus='0' target='0' unit='2'/> + </disk> + <controller type='usb' index='0'/> + <controller type='scsi' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <graphics type='sdl'/> + <video> + <model type='cirrus' vram='9216' heads='1'/> + </video> + <memballoon model='virtio'/> + </devices> +</domain>

3 2

[libvirt] Planning for next release
by Daniel Veillard 22 Nov '13

22 Nov '13

If we want to land a release on Mon Dec 2, I would suggest entering freeze next week. The amount of patches since 1.1.4 is not very large so we could start the freeze say on Wed 27, but IMHO it all depennds if Dan patches about splitting out the python bindings gets in for that release. If yes I would probably prefer to freeze one more day (start Tues 26), and bump release name to 1.2.0 as this is a significant change from an user perspective, otherwise 1.1.5 and freeze next Wed. Opinions ? Dan do you think you can/want to land this set in the coming week ? Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/

5 10

[libvirt] [PATCH] Mostly revert "python: remove virConnectGetCPUModelNames from globals"
by Daniel P. Berrange 22 Nov '13

22 Nov '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> This reverts commit 6b90d7428d72e92db292a9228c44701bfd5003c9. The original problem was that libvirt_virConnectGetCPUModelNames was listed twice in the exports table, once automatically from the generator and once from the manual override. We merely needed to list it in the skip_impl list, and not delete the manually written code entirely. --- python/libvirt-override.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 2f520c1..6546dd1 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -2276,6 +2276,58 @@ libvirt_virConnectGetVersion(PyObject *self ATTRIBUTE_UNUSED, return PyInt_FromLong(hvVersion); } +PyObject * +libvirt_virConnectGetCPUModelNames(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + int c_retval; + virConnectPtr conn; + PyObject *rv = NULL, *pyobj_conn; + char **models = NULL; + size_t i; + int flags = 0; + const char *arch = NULL; + + if (!PyArg_ParseTuple(args, (char *)"Osi:virConnectGetCPUModelNames", + &pyobj_conn, &arch, &flags)) + return NULL; + conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn); + + LIBVIRT_BEGIN_ALLOW_THREADS; + + c_retval = virConnectGetCPUModelNames(conn, arch, &models, flags); + + LIBVIRT_END_ALLOW_THREADS; + + if (c_retval == -1) + return VIR_PY_INT_FAIL; + + if ((rv = PyList_New(c_retval)) == NULL) + goto error; + + for (i = 0; i < c_retval; i++) { + PyObject *str; + if ((str = PyString_FromString(models[i])) == NULL) + goto error; + + PyList_SET_ITEM(rv, i, str); + } + +done: + if (models) { + for (i = 0; i < c_retval; i++) + VIR_FREE(models[i]); + VIR_FREE(models); + } + + return rv; + +error: + Py_XDECREF(rv); + rv = VIR_PY_INT_FAIL; + goto done; +} + static PyObject * libvirt_virConnectGetLibVersion(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) @@ -7176,6 +7228,7 @@ static PyMethodDef libvirtMethods[] = { #include "libvirt-export.c" {(char *) "virGetVersion", libvirt_virGetVersion, METH_VARARGS, NULL}, {(char *) "virConnectGetVersion", libvirt_virConnectGetVersion, METH_VARARGS, NULL}, + {(char *) "virConnectGetCPUModelNames", libvirt_virConnectGetCPUModelNames, METH_VARARGS, NULL}, {(char *) "virConnectGetLibVersion", libvirt_virConnectGetLibVersion, METH_VARARGS, NULL}, {(char *) "virConnectOpenAuth", libvirt_virConnectOpenAuth, METH_VARARGS, NULL}, {(char *) "virConnectListDomainsID", libvirt_virConnectListDomainsID, METH_VARARGS, NULL}, -- 1.8.3.1

2 1

[libvirt] [PATCH] Don't start a nested job in qemuMigrationPrepareAny
by Ján Tomko 22 Nov '13

22 Nov '13

This nested job is canceled by the first ExitMonitor call (even though it was not created by the corresponding EnterMonitor call), and again in qemuMigrationPrepareAny if qemuProcessStart failed. This can lead to a crash if the vm object was disposed of before calling qemuDomainRemoveInactive: 0 ..62bc in virClassIsDerivedFrom (klass=0xdeadbeef, parent=0x7ffce4cdd270) at util/virobject.c:166 1 ..6666 in virObjectIsClass at util/virobject.c:362 2 ..66b4 in virObjectLock at util/virobject.c:314 3 ..477e in virDomainObjListRemove at conf/domain_conf.c:2359 4 ..7a64 in qemuDomainRemoveInactive at qemu/qemu_domain.c:2087 5 ..956c in qemuMigrationPrepareAny at qemu/qemu_migration.c:2469 This was added by commit e4e2822, exposed by 5a4c237 and c7ac251. https://bugzilla.redhat.com/show_bug.cgi?id=1018267 --- src/qemu/qemu_migration.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e87ea85..ef6f1c5 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2358,10 +2358,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, goto endjob; } - if (qemuDomainObjBeginNestedJob(driver, vm, - QEMU_ASYNC_JOB_MIGRATION_IN) < 0) - goto endjob; - /* Start the QEMU daemon, with the same command-line arguments plus * -incoming $migrateFrom */ @@ -2370,8 +2366,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, VIR_QEMU_PROCESS_START_PAUSED | VIR_QEMU_PROCESS_START_AUTODESTROY) < 0) { virDomainAuditStart(vm, "migrated", false); - if (!qemuDomainObjEndJob(driver, vm)) - vm = NULL; goto endjob; } @@ -2474,7 +2468,7 @@ stop: qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, 0); endjob: - if (vm && !qemuMigrationJobFinish(driver, vm)) { + if (!qemuMigrationJobFinish(driver, vm)) { vm = NULL; } goto cleanup; -- 1.8.3.2

2 2

[libvirt] [PATCH] util: Make port allocator data more up-to-date
by Martin Kletzander 22 Nov '13

22 Nov '13

There are few places where we know about port getting opened even though it was not allocated on any virPortAllocator (or we know it was reserved like this). We cannot notify the virPortAllocator that the port is open because of that (since the port may be out of range) and in that case, we are stuck with old data. This also fixes the issue that if you have autoport='no' and port='-1', the port never gets returned (introduced by 7b4a630484); the case when we don't know whether to release the port or not. There may be also other places where the port is being released even though it might have been set statically out of range by the user. In that case we report an error, but that shouldn't stop APIs releasing ports (e.g. destroy). Moreover, the return value is not checked sometimes and we're stuck with an error in the logs and also set as a lastError. To make this behave the best I can imagine, we'd have to have a single virPortAllocator with configurable ranges (ports shared between allocators), but that's an idea for another day. In this patch, I'm modifyping the virPortAllocatorRelease() function to be safe for all kinds of parameters, making it a no-op in case of out-of-range ports and warning in case virBitmapClearBit() returns an error (which is just a sanity code in combination with the range check). Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/libxl/libxl_driver.c | 13 ++++--------- src/qemu/qemu_process.c | 32 +++++++++++++++++++++----------- src/util/virportallocator.c | 33 +++++++++------------------------ src/util/virportallocator.h | 4 ++-- tests/virportallocatortest.c | 5 +---- 5 files changed, 37 insertions(+), 50 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 1b42f14..26e2d3a 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -279,15 +279,10 @@ libxlVmCleanup(libxlDriverPrivatePtr driver, if (virAtomicIntDecAndTest(&driver->nactive) && driver->inhibitCallback) driver->inhibitCallback(false, driver->inhibitOpaque); - if ((vm->def->ngraphics == 1) && - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && - vm->def->graphics[0]->data.vnc.autoport) { - vnc_port = vm->def->graphics[0]->data.vnc.port; - if (vnc_port >= LIBXL_VNC_PORT_MIN) { - if (virPortAllocatorRelease(driver->reservedVNCPorts, - vnc_port) < 0) - VIR_DEBUG("Could not mark port %d as unused", vnc_port); - } + if (vm->def->ngraphics == 1 && + vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + virPortAllocatorRelease(driver->reservedVNCPorts, + vm->def->graphics[0]->data.vnc.port); } /* Remove any cputune settings */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a26c079..dfaf95f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4331,25 +4331,35 @@ retry: qemuProcessRemoveDomainStatus(driver, vm); - /* Remove VNC and Spice ports from port reservation bitmap, but only if - they were reserved by the driver (autoport=yes) - */ + /* Remove remote ports from all port allocators, even when + * autoport isn't set, because they might have been auto-allocated + * anyway. It also helps keeping the allocator data a tad more + * updated. + */ for (i = 0; i < vm->def->ngraphics; ++i) { virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; - if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { - if (graphics->data.vnc.autoport) { - virPortAllocatorRelease(driver->remotePorts, - graphics->data.vnc.port); - } + switch ((enum virDomainGraphicsType) graphics->type) { + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + virPortAllocatorRelease(driver->remotePorts, + graphics->data.vnc.port); virPortAllocatorRelease(driver->webSocketPorts, graphics->data.vnc.websocket); - } - if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && - graphics->data.spice.autoport) { + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: virPortAllocatorRelease(driver->remotePorts, graphics->data.spice.port); virPortAllocatorRelease(driver->remotePorts, graphics->data.spice.tlsPort); + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + case VIR_DOMAIN_GRAPHICS_TYPE_RDP: + case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: + /* Nothing has been allocated */ + + case VIR_DOMAIN_GRAPHICS_TYPE_LAST: + break; } } diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index 694f191..f7f3755 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -27,11 +27,12 @@ #include "viralloc.h" #include "virbitmap.h" -#include "virportallocator.h" -#include "virthread.h" #include "virerror.h" #include "virfile.h" +#include "virlog.h" +#include "virportallocator.h" #include "virstring.h" +#include "virthread.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -172,33 +173,17 @@ cleanup: return ret; } -int virPortAllocatorRelease(virPortAllocatorPtr pa, - unsigned short port) +void virPortAllocatorRelease(virPortAllocatorPtr pa, + unsigned short port) { - int ret = -1; - if (!port) - return 0; + return; virObjectLock(pa); - if (port < pa->start || - port > pa->end) { - virReportInvalidArg(port, "port %d must be in range (%d, %d)", - port, pa->start, pa->end); - goto cleanup; - } - - if (virBitmapClearBit(pa->bitmap, - port - pa->start) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to release port %d"), - port); - goto cleanup; - } + if (port >= pa->start && port <= pa->end && + virBitmapClearBit(pa->bitmap, port - pa->start) < 0) + VIR_WARN("Problem releasing port %d from range '%s'", port, pa->name); - ret = 0; -cleanup: virObjectUnlock(pa); - return ret; } diff --git a/src/util/virportallocator.h b/src/util/virportallocator.h index c8aa6de..ba7c504 100644 --- a/src/util/virportallocator.h +++ b/src/util/virportallocator.h @@ -35,7 +35,7 @@ virPortAllocatorPtr virPortAllocatorNew(const char *name, int virPortAllocatorAcquire(virPortAllocatorPtr pa, unsigned short *port); -int virPortAllocatorRelease(virPortAllocatorPtr pa, - unsigned short port); +void virPortAllocatorRelease(virPortAllocatorPtr pa, + unsigned short port); #endif /* __VIR_PORT_ALLOCATOR_H__ */ diff --git a/tests/virportallocatortest.c b/tests/virportallocatortest.c index 721356e..0d41c98 100644 --- a/tests/virportallocatortest.c +++ b/tests/virportallocatortest.c @@ -165,10 +165,7 @@ static int testAllocReuse(const void *args ATTRIBUTE_UNUSED) goto cleanup; } - - if (virPortAllocatorRelease(alloc, p2) < 0) - goto cleanup; - + virPortAllocatorRelease(alloc, p2); if (virPortAllocatorAcquire(alloc, &p4) < 0) goto cleanup; if (p4 != 5902) { -- 1.8.4.3

3 3

[libvirt] [PATCH] spec: Don't save/restore running VMs on libvirt-client update
by Cole Robinson 22 Nov '13

22 Nov '13

Restarting an active libvirt-guests.service is the equivalent of doing: /usr/libexec/libvirt-guests.sh stop /usr/libexec/libvirt-guests.sh start Which in a default configuration will managedsave every running VM, and then restore them. Certainly not something we should do every time the libvirt-client RPM is updated. Just drop the try-restart attempt, I don't know what purpose it serves anyways. https://bugzilla.redhat.com/show_bug.cgi?id=962225 --- libvirt.spec.in | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index a5b01df..0d50ccd 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1714,9 +1714,8 @@ fi # If the package is allowed to autostart: /bin/systemctl --no-reload enable libvirt-guests.service >/dev/null 2>&1 ||: -# Run these because the SysV package being removed won't do them +# Run this because the SysV package being removed won't do them /sbin/chkconfig --del libvirt-guests >/dev/null 2>&1 || : -/bin/systemctl try-restart libvirt-guests.service >/dev/null 2>&1 || : %endif %if %{with_sanlock} -- 1.8.4.2

2 2

[libvirt] [PATCH] spec: Restrict virt-login-shell usage
by Jiri Denemark 22 Nov '13

22 Nov '13

https://bugzilla.redhat.com/show_bug.cgi?id=1033614 As virt-login-shell is an SUID binary, we should restrict its usage to just the users chosen by an administrator to use virt-login-shell as their login shell. This can easily be done by making the binary executable only by users from a new virtlogin group. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- libvirt.spec.in | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index a5b01df..864fbf4 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1727,6 +1727,12 @@ if getent group sanlock > /dev/null ; then fi %endif +%if %{with_lxc} +%pre login-shell +getent group virtlogin >/dev/null || groupadd -r virtlogin +exit 0 +%endif + %files %defattr(-, root, root) @@ -2072,7 +2078,7 @@ fi %if %{with_lxc} %files login-shell -%attr(4755, root, root) %{_bindir}/virt-login-shell +%attr(4750, root, virtlogin) %{_bindir}/virt-login-shell %config(noreplace) %{_sysconfdir}/libvirt/virt-login-shell.conf %{_mandir}/man1/virt-login-shell.1* %endif -- 1.8.4.4

2 2