- Devel - libvirt List Archives

[libvirt] [PATCH] qemudDomainSetVcpus: avoid NULL-deref
by Jim Meyering 15 May '10

15 May '10

Here's a patch for a coverity-spotted bug: >From 12160fa54bc948e5de3fecff6a9552995e9595b0 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 14 May 2010 12:38:43 +0200 Subject: [PATCH] qemudDomainSetVcpus: avoid NULL-deref * src/qemu/qemu_driver.c (qemudDomainSetVcpus): Avoid NULL-deref upon unknown UUID. Call qemuDomainObjBeginJob(vm) only after ensuring that vm != NULL, not before. This potential NULL-deref was introduced by commit 2c555d87b0041e0d1ec4742386d2161d1b2f0600. --- src/qemu/qemu_driver.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bb1079e..cbddb96 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5410,9 +5410,6 @@ static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { vm = virDomainFindByUUID(&driver->domains, dom->uuid); qemuDriverUnlock(driver); - if (qemuDomainObjBeginJob(vm) < 0) - goto cleanup; - if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(dom->uuid, uuidstr); @@ -5421,6 +5418,9 @@ static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { goto endjob; } + if (qemuDomainObjBeginJob(vm) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); -- 1.7.1.460.gf3c4c

2 2

[libvirt] [PATCH] Fix a misuse of virAsprintf in qemudDomainMemoryPeek
by Ryota Ozaki 14 May '10

14 May '10

The code specifies driver->cacheDir as the format string, but it usually doesn't contain '%s', so the subsequent argument, "/qemu.mem.XXXXXX", is always ignored. The patch fixes the misuse. --- src/qemu/qemu_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bb1079e..843f827 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9033,7 +9033,7 @@ qemudDomainMemoryPeek (virDomainPtr dom, goto endjob; } - if (virAsprintf(&tmp, driver->cacheDir, "/qemu.mem.XXXXXX") < 0) { + if (virAsprintf(&tmp, "%s/qemu.mem.XXXXXX", driver->cacheDir) < 0) { virReportOOMError(); goto endjob; } -- 1.6.5.2

2 2

[libvirt] How many domains can be created and run in parallel in a Host.
by Kumar L Srikanth-B22348 14 May '10

14 May '10

Hi, I have a general doubt regarding Virtual Domains in libvirt. How many Virtual Domains can be created and run in parallel [at max] in a host machine using libvirt. Regards, Srikanth.

2 1

[libvirt] [PATCH] Query block allocation extent from QEMU monitor
by Daniel P. Berrange 14 May '10

14 May '10

The virDomainGetBlockInfo API allows query physical block extent and allocated block extent. These are normally the same value unless storing a special format like qcow2 inside a block device. In this scenario we can query QEMU to get the actual allocated extent. * src/qemu/qemu_driver.c: Fill in block aloction extent when VM is running * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h, src/qemu/qemu_monitor_json.c, src/qemu/qemu_monitor_json.h, src/qemu/qemu_monitor_text.c, src/qemu/qemu_monitor_text.h: Add API to query the highest block extent via info blockstats --- src/qemu/qemu_driver.c | 32 +++++++++++--- src/qemu/qemu_monitor.c | 16 +++++++ src/qemu/qemu_monitor.h | 4 ++ src/qemu/qemu_monitor_json.c | 97 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 + src/qemu/qemu_monitor_text.c | 12 +++++ src/qemu/qemu_monitor_text.h | 4 +- 7 files changed, 160 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5089129..4251a66 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9651,6 +9651,7 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, int fd = -1; off_t end; virStorageFileMetadata meta; + virDomainDiskDefPtr disk = NULL; struct stat sb; int i; @@ -9677,19 +9678,17 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, for (i = 0 ; i < vm->def->ndisks ; i++) { if (vm->def->disks[i]->src != NULL && STREQ (vm->def->disks[i]->src, path)) { - ret = 0; + disk = vm->def->disks[i]; break; } } - if (ret != 0) { + if (!disk) { qemuReportError(VIR_ERR_INVALID_ARG, _("invalid path %s not assigned to domain"), path); goto cleanup; } - ret = -1; - /* The path is correct, now try to open it and get its size. */ fd = open (path, O_RDONLY); if (fd == -1) { @@ -9740,11 +9739,30 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, if (meta.capacity) info->capacity = meta.capacity; - /* XXX allocation will need to be pulled from QEMU for - * the qcow inside LVM case */ + /* Set default value .. */ info->allocation = info->physical; - ret = 0; + /* ..but if guest is running & not using raw + disk format and on a block device, then query + highest allocated extent from QEMU */ + if (virDomainObjIsActive(vm) && + meta.format != VIR_STORAGE_FILE_RAW && + S_ISBLK(sb.st_mode)) { + qemuDomainObjPrivatePtr priv = vm->privateData; + if (qemuDomainObjBeginJob(vm) < 0) + goto cleanup; + + qemuDomainObjEnterMonitor(vm); + ret = qemuMonitorGetBlockExtent(priv->mon, + disk->info.alias, + &info->allocation); + qemuDomainObjExitMonitor(vm); + + if (qemuDomainObjEndJob(vm) == 0) + vm = NULL; + } else { + ret = 0; + } cleanup: if (fd != -1) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index f77ec44..4a77f39 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1000,6 +1000,22 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, return ret; } +int qemuMonitorGetBlockExtent(qemuMonitorPtr mon, + const char *devname, + unsigned long long *extent) +{ + int ret; + DEBUG("mon=%p, fd=%d, devname=%p", + mon, mon->fd, devname); + + if (mon->json) + ret = qemuMonitorJSONGetBlockExtent(mon, devname, extent); + else + ret = qemuMonitorTextGetBlockExtent(mon, devname, extent); + + return ret; +} + int qemuMonitorSetVNCPassword(qemuMonitorPtr mon, const char *password) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 7b1589e..adfb3bc 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -185,6 +185,10 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, long long *wr_bytes, long long *errs); +int qemuMonitorGetBlockExtent(qemuMonitorPtr mon, + const char *devname, + unsigned long long *extent); + int qemuMonitorSetVNCPassword(qemuMonitorPtr mon, const char *password); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 6d8f328..a15609c 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1186,6 +1186,103 @@ cleanup: } +int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, + const char *devname, + unsigned long long *extent) +{ + int ret; + int i; + int found = 0; + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-blockstats", + NULL); + virJSONValuePtr reply = NULL; + virJSONValuePtr devices; + + *extent = 0; + + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) { + ret = qemuMonitorJSONCheckError(cmd, reply); + if (ret < 0) + goto cleanup; + } + ret = -1; + + devices = virJSONValueObjectGet(reply, "return"); + if (!devices || devices->type != VIR_JSON_TYPE_ARRAY) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats reply was missing device list")); + goto cleanup; + } + + for (i = 0 ; i < virJSONValueArraySize(devices) ; i++) { + virJSONValuePtr dev = virJSONValueArrayGet(devices, i); + virJSONValuePtr stats; + virJSONValuePtr parent; + const char *thisdev; + if (!dev || dev->type != VIR_JSON_TYPE_OBJECT) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats device entry was not in expected format")); + goto cleanup; + } + + if ((thisdev = virJSONValueObjectGetString(dev, "device")) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats device entry was not in expected format")); + goto cleanup; + } + + /* New QEMU has separate names for host & guest side of the disk + * and libvirt gives the host side a 'drive-' prefix. The passed + * in devname is the guest side though + */ + if (STRPREFIX(thisdev, QEMU_DRIVE_HOST_PREFIX)) + thisdev += strlen(QEMU_DRIVE_HOST_PREFIX); + + if (STRNEQ(thisdev, devname)) + continue; + + found = 1; + if ((parent = virJSONValueObjectGet(dev, "parent")) == NULL || + parent->type != VIR_JSON_TYPE_OBJECT) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats parent entry was not in expected format")); + goto cleanup; + } + + if ((stats = virJSONValueObjectGet(parent, "stats")) == NULL || + stats->type != VIR_JSON_TYPE_OBJECT) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats stats entry was not in expected format")); + goto cleanup; + } + + if (virJSONValueObjectGetNumberUlong(stats, "wr_highest_offset", extent) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s statistic"), + "wr_highest_offset"); + goto cleanup; + } + } + + if (!found) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find statistics for device '%s'"), devname); + goto cleanup; + } + ret = 0; + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + int qemuMonitorJSONSetVNCPassword(qemuMonitorPtr mon, const char *password) { diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 26fc865..14597f4 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -56,6 +56,9 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, long long *wr_req, long long *wr_bytes, long long *errs); +int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, + const char *devname, + unsigned long long *extent); int qemuMonitorJSONSetVNCPassword(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index d725d6d..19038d1 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -711,6 +711,18 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, } +int qemuMonitorTextGetBlockExtent(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + const char *devname ATTRIBUTE_UNUSED, + unsigned long long *extent) +{ + /* Not supported in text monitor, but we don't want to + * cause an error in callers in this scenario, just + * fallback to marking the data unavailable */ + *extent = 0; + return 0; +} + + static int qemuMonitorSendVNCPassphrase(qemuMonitorPtr mon ATTRIBUTE_UNUSED, qemuMonitorMessagePtr msg, diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index 2a62c7e..6fb7d7a 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -55,7 +55,9 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, long long *wr_req, long long *wr_bytes, long long *errs); - +int qemuMonitorTextGetBlockExtent(qemuMonitorPtr mon, + const char *devname, + unsigned long long *extent); int qemuMonitorTextSetVNCPassword(qemuMonitorPtr mon, const char *password); -- 1.6.6.1

2 2

[libvirt] CAP_SYS_RAWIO missing for qemu-kvm device passthrough
by Gerd v. Egidy 14 May '10

14 May '10

Hi, I'm running current git libvirt on Fedora 13 beta. I enabled the use of libcap-ng as it is done in the regular F13 .spec. When I now pass a pci card through to a qemu-kvm guest using vt-d I get this error from qemu-kvm: Failed to assign irq for "hostdev0": Operation not permitted Perhaps you are assigning a device that shares an IRQ with another device? I'm running qemu-kvm as root. But that doesn't seem to be enough: I traced the issue down to a missing CAP_SYS_RAWIO.The kvm kernel module requires CAP_SYS_RAWIO to use the KVM_ASSIGN_DEV_IRQ ioctl. When I remove the capability-dropping from libvirt like this everything works as expected: --- libvirt/src/qemu/qemu_driver.c 2010-05-13 22:50:13.000000000 +0200 +++ libvirt.new/src/qemu/qemu_driver.c 2010-05-13 23:18:49.286311290 +0200 @@ -3359,7 +3359,7 @@ ret = virExecDaemonize(argv, progenv, &keepfd, &child, stdin_fd, &logfile, &logfile, - VIR_EXEC_NONBLOCK | VIR_EXEC_CLEAR_CAPS, + VIR_EXEC_NONBLOCK, qemudSecurityHook, &hookData, pidfile); VIR_FREE(pidfile); Is there a better solution to get device passthrough to work? Kind regards, Gerd

3 4

[libvirt] libvirt-java
by Stephen Shaw 13 May '10

13 May '10

I was wondering how long it usually takes for the java bindings to be updated to the latest version of libvirt. I'm interested in the newer kvm snapshotting features specifically. Thanks, Stephen

4 6

[libvirt] [PATCH] Make domain save work when dynamic_ownership=0
by Daniel P. Berrange 13 May '10

13 May '10

Setting dynamic_ownership=0 in /etc/libvirt/qemu.conf prevents libvirt's DAC security driver from setting uid/gid on disk files when starting/stopping QEMU, allowing the admin to manage this manually. As a side effect it also stopped setting of uid/gid when saving guests to a file, which completely breaks save when QEMU is running non-root. Thus saved state labelling code must ignore the dynamic_ownership parameter * src/qemu/qemu_security_dac.c: Ignore dynamic_ownership=0 when doing save/restore image labelling --- src/qemu/qemu_security_dac.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_security_dac.c b/src/qemu/qemu_security_dac.c index 2d42ce2..364227d 100644 --- a/src/qemu/qemu_security_dac.c +++ b/src/qemu/qemu_security_dac.c @@ -407,7 +407,7 @@ static int qemuSecurityDACSetSavedStateLabel(virDomainObjPtr vm ATTRIBUTE_UNUSED, const char *savefile) { - if (!driver->privileged || !driver->dynamicOwnership) + if (!driver->privileged) return 0; return qemuSecurityDACSetOwnership(savefile, driver->user, driver->group); @@ -418,7 +418,7 @@ static int qemuSecurityDACRestoreSavedStateLabel(virDomainObjPtr vm ATTRIBUTE_UNUSED, const char *savefile) { - if (!driver->privileged || !driver->dynamicOwnership) + if (!driver->privileged) return 0; return qemuSecurityDACRestoreSecurityFileLabel(savefile); -- 1.6.6.1

2 1

[libvirt] [PATCH v3] vepa: parsing for 802.1Qb{g|h} XML
by Stefan Berger 13 May '10

13 May '10

Below is David Alan's original patch with lots of changes. In particular, it now parses the following two XML descriptions, one for 802.1Qbg and 802.1Qbh and stored the data internally. The actual triggering of the switch setup protocol has not been implemented here but the relevant code to do that should go into the functions associatePortProfileId() and disassociatePortProfileId(). <interface type='direct'> <source dev='static' mode='vepa'/> <model type='virtio'/> <vsi type='802.1Qbg'> <parameters managerid='12' typeid='0x123456' typeidversion='1' instanceid='fa9b7fff-b0a0-4893-8e0e-beef4ff18f8f'/> </vsi> <filterref filter='clean-traffic'/> </interface> <interface type='direct'> <source dev='static' mode='vepa'/> <model type='virtio'/> <vsi type='802.1Qbh'> <parameters profileid='my_profile'/> </vsi> </interface> I'd suggest to use this patch as a base for triggering the setup protocol with the 802.1Qb{g|h} switch. Changes from V2 to V3: - if instance ID UUID is not supplied it will automatically be generated - adapted schema to make instance ID UUID optional - added test case Some of the changes from V1 to V2: - parser and XML generator have been separated into their own functions so they can be re-used elsewhere (passthrough case for example) - Adapted XML parser and generator support the above shown type (802.1Qbg, 802.1Qbh). - Adapted schema to above XML - Adapted test XML to above XML - Passing through the VM's UUID which seems to be necessary for 802.1Qbh -- sorry no host UUID - adding virtual function ID to association function, in case it's necessary to use (for SR-IOV) Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> >From a945107f047c7cd71f9c1b74fd74c47d8cdc3670 Mon Sep 17 00:00:00 2001 From: David Allan <dallan(a)redhat.com> Date: Fri, 12 Mar 2010 13:25:04 -0500 Subject: [PATCH 1/1] POC of port profile id support * Modified schema per DanPB's feedback * Added test for modified schema --- docs/schemas/domain.rng | 69 ++++++++++++++ src/conf/domain_conf.c | 155 +++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 35 +++++++ src/qemu/qemu_conf.c | 17 +-- src/qemu/qemu_conf.h | 5 - src/qemu/qemu_driver.c | 17 +-- src/util/macvtap.c | 150 ++++++++++++++++++++++++++++--- src/util/macvtap.h | 10 +- tests/domainschemadata/portprofile.xml | 36 +++++++ 9 files changed, 446 insertions(+), 48 deletions(-) create mode 100644 tests/domainschemadata/portprofile.xml Index: libvirt-acl/docs/schemas/domain.rng =================================================================== --- libvirt-acl.orig/docs/schemas/domain.rng +++ libvirt-acl/docs/schemas/domain.rng @@ -817,6 +817,9 @@ </optional> <empty/> </element> + <optional> + <ref name="vsiProfile"/> + </optional> <ref name="interface-options"/> </interleave> </group> @@ -902,6 +905,45 @@ </optional> </interleave> </define> + <define name="vsiProfile"> + <choice> + <group> + <element name="vsi"> + <attribute name="type"> + <value>802.1Qbg</value> + </attribute> + <element name="parameters"> + <attribute name="managerid"> + <ref name="uint8range"/> + </attribute> + <attribute name="typeid"> + <ref name="uint24range"/> + </attribute> + <attribute name="typeidversion"> + <ref name="uint8range"/> + </attribute> + <optional> + <attribute name="instanceid"> + <ref name="UUID"/> + </attribute> + </optional> + </element> + </element> + </group> + <group> + <element name="vsi"> + <attribute name="type"> + <value>802.1Qbh</value> + </attribute> + <element name="parameters"> + <attribute name="profileid"> + <ref name="vsiProfileID"/> + </attribute> + </element> + </element> + </group> + </choice> + </define>  @@ -1769,4 +1811,31 @@ <param name="pattern">[a-zA-Z0-9_\.:]+</param> </data> </define> + <define name="uint8range"> + <choice> + <data type="string"> + <param name="pattern">0x[0-9a-fA-F]{1,2}</param> + </data> + <data type="int"> + <param name="minInclusive">0</param> + <param name="maxInclusive">255</param> + </data> + </choice> + </define> + <define name="uint24range"> + <choice> + <data type="string"> + <param name="pattern">0x[0-9a-fA-F]{1,6}</param> + </data> + <data type="int"> + <param name="minInclusive">0</param> + <param name="maxInclusive">16777215</param> + </data> + </choice> + </define> + <define name="vsiProfileID"> + <data type="string"> + <param name="maxLength">39</param> + </data> + </define> </grammar> Index: libvirt-acl/src/conf/domain_conf.c =================================================================== --- libvirt-acl.orig/src/conf/domain_conf.c +++ libvirt-acl/src/conf/domain_conf.c @@ -242,6 +242,11 @@ VIR_ENUM_IMPL(virDomainNetdevMacvtap, VI "private", "bridge") +VIR_ENUM_IMPL(virVSI, VIR_VSI_TYPE_LAST, + "none", + "802.1Qbg", + "802.1Qbh") + VIR_ENUM_IMPL(virDomainClockOffset, VIR_DOMAIN_CLOCK_OFFSET_LAST, "utc", "localtime", @@ -1807,6 +1812,145 @@ isValidIfname(const char *ifname) { } +static void +virVSIProfileDefParseXML(xmlNodePtr node, + virVSIProfileDefPtr vsi) +{ + char *vsiType; + char *vsiManagerID = NULL; + char *vsiTypeID = NULL; + char *vsiTypeIDVersion = NULL; + char *vsiInstanceID = NULL; + char *vsiProfileID = NULL; + xmlNodePtr cur = node->children; + + vsiType = virXMLPropString(node, "type"); + if (!vsiType) + return; + + while (cur != NULL) { + if (xmlStrEqual(cur->name, BAD_CAST "parameters")) { + + vsiManagerID = virXMLPropString(cur, "managerid"); + vsiTypeID = virXMLPropString(cur, "typeid"); + vsiTypeIDVersion = virXMLPropString(cur, "typeidversion"); + vsiInstanceID = virXMLPropString(cur, "instanceid"); + vsiProfileID = virXMLPropString(cur, "profileid"); + + break; + } + + cur = cur->next; + } + + vsi->vsiType = VIR_VSI_NONE; + + switch (virVSITypeFromString(vsiType)) { + + case VIR_VSI_8021QBG: + if (vsiManagerID != NULL && vsiTypeID != NULL && + vsiTypeIDVersion != NULL) { + unsigned int val; + + if ((virStrToLong_ui(vsiManagerID, NULL, 10, &val) && + virStrToLong_ui(vsiManagerID, NULL, 16, &val) ) || + val > 0xff) + break; + + vsi->u.vsi8021Qbg.managerID = (uint8_t)val; + + if ((virStrToLong_ui(vsiTypeID, NULL, 10, &val) && + virStrToLong_ui(vsiTypeID, NULL, 16, &val) ) || + val > 0xffffff) + break; + + vsi->u.vsi8021Qbg.typeID = (uint32_t)val; + + if ((virStrToLong_ui(vsiTypeIDVersion, NULL, 10, &val) && + virStrToLong_ui(vsiTypeIDVersion, NULL, 16, &val) ) || + val > 0xff) + break; + + vsi->u.vsi8021Qbg.typeIDVersion = (uint8_t)val; + + if (vsiInstanceID != NULL) { + if (virUUIDParse(vsiInstanceID, vsi->u.vsi8021Qbg.instanceID)) + break; + } else { + if (virUUIDGenerate(vsi->u.vsi8021Qbg.instanceID)) + break; + } + + vsi->vsiType = VIR_VSI_8021QBG; + } + break; + + case VIR_VSI_8021QBH: + if (vsiProfileID != NULL) { + if (virStrcpyStatic(vsi->u.vsi8021Qbh.profileID, + vsiProfileID) != NULL) + vsi->vsiType = VIR_VSI_8021QBH; + } + break; + + + default: + case VIR_VSI_NONE: + case VIR_VSI_TYPE_LAST: + break; + } + + VIR_FREE(vsiManagerID); + VIR_FREE(vsiTypeID); + VIR_FREE(vsiTypeIDVersion); + VIR_FREE(vsiInstanceID); + VIR_FREE(vsiProfileID); + VIR_FREE(vsiType); +} + + +static void +virVSIProfileFormat(virBufferPtr buf, virVSIProfileDefPtr vsi, + const char *indent) +{ + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + if (vsi->vsiType == VIR_VSI_NONE) + return; + + virBufferVSprintf(buf, "%s<vsi type='%s'>\n", + indent, virVSITypeToString(vsi->vsiType)); + + switch (vsi->vsiType) { + case VIR_VSI_NONE: + case VIR_VSI_TYPE_LAST: + break; + + case VIR_VSI_8021QBG: + virUUIDFormat(vsi->u.vsi8021Qbg.instanceID, + uuidstr); + virBufferVSprintf(buf, + "%s <parameters managerid='%d' typeid='%d' " + "typeidversion='%d' instanceid='%s'/>\n", + indent, + vsi->u.vsi8021Qbg.managerID, + vsi->u.vsi8021Qbg.typeID, + vsi->u.vsi8021Qbg.typeIDVersion, + uuidstr); + break; + + case VIR_VSI_8021QBH: + virBufferVSprintf(buf, + "%s <parameters profileid='%s'/>\n", + indent, + vsi->u.vsi8021Qbh.profileID); + break; + } + + virBufferVSprintf(buf, "%s</vsi>\n", indent); +} + + /* Parse the XML definition for a network interface * @param node XML nodeset to parse for net definition * @return 0 on success, -1 on failure @@ -1832,6 +1976,8 @@ virDomainNetDefParseXML(virCapsPtr caps, char *devaddr = NULL; char *mode = NULL; virNWFilterHashTablePtr filterparams = NULL; + virVSIProfileDef vsi; + bool vsiParsed = false; if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -1873,6 +2019,11 @@ virDomainNetDefParseXML(virCapsPtr caps, xmlStrEqual(cur->name, BAD_CAST "source")) { dev = virXMLPropString(cur, "dev"); mode = virXMLPropString(cur, "mode"); + } else if ((vsiParsed == false) && + (def->type == VIR_DOMAIN_NET_TYPE_DIRECT) && + xmlStrEqual(cur->name, BAD_CAST "vsi")) { + virVSIProfileDefParseXML(cur, &vsi); + vsiParsed = true; } else if ((network == NULL) && ((def->type == VIR_DOMAIN_NET_TYPE_SERVER) || (def->type == VIR_DOMAIN_NET_TYPE_CLIENT) || @@ -2049,6 +2200,8 @@ virDomainNetDefParseXML(virCapsPtr caps, } else def->data.direct.mode = VIR_DOMAIN_NETDEV_MACVTAP_MODE_VEPA; + def->data.direct.vsiProfile = vsi; + def->data.direct.linkdev = dev; dev = NULL; @@ -5141,6 +5294,8 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferVSprintf(buf, " mode='%s'", virDomainNetdevMacvtapTypeToString(def->data.direct.mode)); virBufferAddLit(buf, "/>\n"); + virVSIProfileFormat(buf, &def->data.direct.vsiProfile, + " "); break; case VIR_DOMAIN_NET_TYPE_USER: Index: libvirt-acl/src/conf/domain_conf.h =================================================================== --- libvirt-acl.orig/src/conf/domain_conf.h +++ libvirt-acl/src/conf/domain_conf.h @@ -259,6 +259,39 @@ enum virDomainNetdevMacvtapType { }; +enum virVSIType { + VIR_VSI_NONE, + VIR_VSI_8021QBG, + VIR_VSI_8021QBH, + + VIR_VSI_TYPE_LAST, +}; + +# ifdef IFLA_VF_PORT_PROFILE_MAX +# define LIBVIRT_IFLA_VF_PORT_PROFILE_MAX IFLA_VF_PORT_PROFILE_MAX +# else +# define LIBVIRT_IFLA_VF_PORT_PROFILE_MAX 40 +# endif + +/* profile data for macvtap (VEPA) */ +typedef struct _virVSIProfileDef virVSIProfileDef; +typedef virVSIProfileDef *virVSIProfileDefPtr; +struct _virVSIProfileDef { + enum virVSIType vsiType; + union { + struct { + uint8_t managerID; + uint32_t typeID; // 24 bit valid + uint8_t typeIDVersion; + unsigned char instanceID[VIR_UUID_BUFLEN]; + } vsi8021Qbg; + struct { + char profileID[LIBVIRT_IFLA_VF_PORT_PROFILE_MAX]; + } vsi8021Qbh; + } u; +}; + + /* Stores the virtual network interface configuration */ typedef struct _virDomainNetDef virDomainNetDef; typedef virDomainNetDef *virDomainNetDefPtr; @@ -290,6 +323,7 @@ struct _virDomainNetDef { struct { char *linkdev; int mode; + virVSIProfileDef vsiProfile; } direct; } data; char *ifname; @@ -1089,6 +1123,7 @@ VIR_ENUM_DECL(virDomainSeclabel) VIR_ENUM_DECL(virDomainClockOffset) VIR_ENUM_DECL(virDomainNetdevMacvtap) +VIR_ENUM_DECL(virVSI) VIR_ENUM_DECL(virDomainTimerName) VIR_ENUM_DECL(virDomainTimerTrack) Index: libvirt-acl/src/util/macvtap.c =================================================================== --- libvirt-acl.orig/src/util/macvtap.c +++ libvirt-acl/src/util/macvtap.c @@ -43,6 +43,7 @@ # include "util.h" # include "memory.h" +# include "logging.h" # include "macvtap.h" # include "interface.h" # include "conf/domain_conf.h" @@ -570,36 +571,127 @@ configMacvtapTap(int tapfd, int vnet_hdr /** + * associatePortProfile + * + * @linkdev: the link dev in case of macvtap + * @mac: the MAC address of the interface + * @mode: The vepa mode (private, bridge or vepa) + * @vsi: pointer to the object holding port profile parameters + * @vf: virtual function number, -1 if to be ignored + * @vmuuid : the UUID of the virtual machine + * + * Associate a port on a swtich with a profile. This function + * may notify a kernel driver or an external daemon to run + * the setup protocol. If profile parameters were not supplied + * by the user, then this function returns without doing + * anything. + * + * Returns 0 in case of success, != 0 otherwise with error + * having been reported. + */ +static int +associatePortProfileId(const char *linkdev, + const unsigned char *mac, + int mode, + const virVSIProfileDefPtr vsi, + int vf, + const unsigned char *vmuuid) +{ + int rc = 0; + VIR_DEBUG("Associating port profile '%p' on link device '%s' mode %d ", + vsi, linkdev, mode); + (void)mac; + (void)vf; + (void)vmuuid; + + switch (vsi->vsiType) { + case VIR_VSI_NONE: + case VIR_VSI_TYPE_LAST: + break; + + case VIR_VSI_8021QBG: + rc = -1; + break; + + case VIR_VSI_8021QBH: + rc = -1; + break; + } + + return rc; +} + + +/** + * disassociatePortProfile + * + * @linkdev: The link dev in case of a macvtap device + * @mac: The MAC address of the interface + * @vsi: point to object holding port profile parameters + * + * Returns 0 in case of success, != 0 otherwise with error + * having been reported. + */ +static int +disassociatePortProfileId(const char *linkdev, + const unsigned char *mac, + const virVSIProfileDefPtr vsi) +{ + int rc = 0; + VIR_DEBUG("Disassociating port profile id '%p' on link device '%s' ", + vsi, linkdev); + (void)mac; + + switch (vsi->vsiType) { + case VIR_VSI_NONE: + case VIR_VSI_TYPE_LAST: + break; + + case VIR_VSI_8021QBG: + rc = -1; + break; + + case VIR_VSI_8021QBH: + rc = -1; + break; + } + + return rc; +} + + +/** * openMacvtapTap: * Create an instance of a macvtap device and open its tap character * device. * @tgifname: Interface name that the macvtap is supposed to have. May * be NULL if this function is supposed to choose a name - * @macaddress: The MAC address for the macvtap device - * @linkdev: The interface name of the NIC to connect to the external bridge - * @mode_str: String describing the mode. Valid are 'bridge', 'vepa' and - * 'private'. + * @net: pointer to the virDomainNetDef object describing the direct + * type if an interface * @res_ifname: Pointer to a string pointer where the actual name of the * interface will be stored into if everything succeeded. It is up * to the caller to free the string. + * @vnet_hdr: Whether to enable IFF_VNET_HDR on the interface + * @vmuuid: The (raw) UUID of the VM * * Returns file descriptor of the tap device in case of success, * negative value otherwise with error reported. * + * Open a macvtap device and trigger the switch setup protocol + * if valid port profile parameters were provided. */ int openMacvtapTap(const char *tgifname, - const unsigned char *macaddress, - const char *linkdev, - int mode, + virDomainNetDefPtr net, char **res_ifname, - int vnet_hdr) + int vnet_hdr, + const unsigned char *vmuuid) { const char *type = "macvtap"; int c, rc; char ifname[IFNAMSIZ]; int retries, do_retry = 0; - uint32_t macvtapMode = macvtapModeFromInt(mode); + uint32_t macvtapMode = macvtapModeFromInt(net->data.direct.mode); const char *cr_ifname; int ifindex; @@ -616,7 +708,7 @@ openMacvtapTap(const char *tgifname, return -1; } cr_ifname = tgifname; - rc = link_add(type, macaddress, 6, tgifname, linkdev, + rc = link_add(type, net->mac, 6, tgifname, net->data.direct.linkdev, macvtapMode, &do_retry); if (rc) return -1; @@ -626,7 +718,8 @@ create_name: for (c = 0; c < 8192; c++) { snprintf(ifname, sizeof(ifname), MACVTAP_NAME_PATTERN, c); if (ifaceGetIndex(false, ifname, &ifindex) == ENODEV) { - rc = link_add(type, macaddress, 6, ifname, linkdev, + rc = link_add(type, net->mac, 6, ifname, + net->data.direct.linkdev, macvtapMode, &do_retry); if (rc == 0) break; @@ -639,6 +732,15 @@ create_name: cr_ifname = ifname; } + rc = associatePortProfileId(net->data.direct.linkdev, + net->mac, + net->data.direct.mode, + &net->data.direct.vsiProfile, + -1, + vmuuid); + if (rc != 0) + goto link_del_exit; + rc = ifaceUp(cr_ifname); if (rc != 0) { virReportSystemError(errno, @@ -647,7 +749,7 @@ create_name: "MAC address"), cr_ifname); rc = -1; - goto link_del_exit; + goto disassociate_exit; } rc = openTap(cr_ifname, 10); @@ -656,14 +758,19 @@ create_name: if (configMacvtapTap(rc, vnet_hdr) < 0) { close(rc); rc = -1; - goto link_del_exit; + goto disassociate_exit; } *res_ifname = strdup(cr_ifname); } else - goto link_del_exit; + goto disassociate_exit; return rc; +disassociate_exit: + disassociatePortProfileId(net->data.direct.linkdev, + net->mac, + &net->data.direct.vsiProfile); + link_del_exit: link_del(cr_ifname); @@ -673,14 +780,21 @@ link_del_exit: /** * delMacvtap: - * @ifname : The name of the macvtap interface + * @net: pointer to virDomainNetDef object * - * Delete an interface given its name. + * Delete an interface given its name. Disassociate + * it with the switch if port profile parameters + * were provided. */ void -delMacvtap(const char *ifname) +delMacvtap(virDomainNetDefPtr net) { - link_del(ifname); + if (net->ifname) { + disassociatePortProfileId(net->data.direct.linkdev, + net->mac, + &net->data.direct.vsiProfile); + link_del(net->ifname); + } } #endif Index: libvirt-acl/src/util/macvtap.h =================================================================== --- libvirt-acl.orig/src/util/macvtap.h +++ libvirt-acl/src/util/macvtap.h @@ -27,15 +27,15 @@ # if defined(WITH_MACVTAP) # include "internal.h" +# include "conf/domain_conf.h" int openMacvtapTap(const char *ifname, - const unsigned char *macaddress, - const char *linkdev, - int mode, + virDomainNetDefPtr net, char **res_ifname, - int vnet_hdr); + int vnet_hdr, + const unsigned char *vmuuid); -void delMacvtap(const char *ifname); +void delMacvtap(virDomainNetDefPtr net); # endif /* WITH_MACVTAP */ Index: libvirt-acl/tests/domainschemadata/portprofile.xml =================================================================== --- /dev/null +++ libvirt-acl/tests/domainschemadata/portprofile.xml @@ -0,0 +1,36 @@ +<domain type='lxc'> + <name>portprofile</name> + <uuid>00000000-0000-0000-0000-000000000000</uuid> + <memory>1048576</memory> + <os> + <type>exe</type> + <init>/sh</init> + </os> + <devices> + <interface type='direct'> + <source dev='eth0' mode='vepa'/> + <vsi type='802.1Qbg'> + <parameters managerid='12' typeid='1193046' typeidversion='1' + instanceid='fa9b7fff-b0a0-4893-8e0e-beef4ff18f8f'/> + </vsi> + </interface> + <interface type='direct'> + <source dev='eth0' mode='vepa'/> + <vsi type='802.1Qbg'> + <parameters managerid='12' typeid='1193046' typeidversion='1'/> + </vsi> + </interface> + <interface type='direct'> + <source dev='eth0' mode='vepa'/> + <vsi type='802.1Qbh'> + <parameters profileid='my_profile'/> + </vsi> + </interface> + <interface type='direct'> + <source dev='eth0' mode='vepa'/> + </interface> + <interface type='direct'> + <source dev='eth0' mode='vepa'/> + </interface> + </devices> +</domain> Index: libvirt-acl/src/qemu/qemu_conf.h =================================================================== --- libvirt-acl.orig/src/qemu/qemu_conf.h +++ libvirt-acl/src/qemu/qemu_conf.h @@ -271,9 +271,8 @@ qemudOpenVhostNet(virDomainNetDefPtr net int qemudPhysIfaceConnect(virConnectPtr conn, struct qemud_driver *driver, virDomainNetDefPtr net, - char *linkdev, - int brmode, - unsigned long long qemuCmdFlags); + unsigned long long qemuCmdFlags, + const unsigned char *vmuuid); int qemudProbeMachineTypes (const char *binary, virCapsGuestMachinePtr **machines, Index: libvirt-acl/src/qemu/qemu_driver.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_driver.c +++ libvirt-acl/src/qemu/qemu_driver.c @@ -3585,10 +3585,8 @@ static void qemudShutdownVMDaemon(struct def = vm->def; for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i]; - if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) { - if (net->ifname) - delMacvtap(net->ifname); - } + if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) + delMacvtap(net); } #endif @@ -7175,9 +7173,8 @@ static int qemudDomainAttachNetDevice(vi } if ((tapfd = qemudPhysIfaceConnect(conn, driver, net, - net->data.direct.linkdev, - net->data.direct.mode, - qemuCmdFlags)) < 0) + qemuCmdFlags, + vm->def->uuid)) < 0) return -1; } @@ -8146,10 +8143,8 @@ qemudDomainDetachNetDevice(struct qemud_ virNWFilterTearNWFilter(detach); #if WITH_MACVTAP - if (detach->type == VIR_DOMAIN_NET_TYPE_DIRECT) { - if (detach->ifname) - delMacvtap(detach->ifname); - } + if (detach->type == VIR_DOMAIN_NET_TYPE_DIRECT) + delMacvtap(detach); #endif if ((driver->macFilter) && (detach->ifname != NULL)) { Index: libvirt-acl/src/qemu/qemu_conf.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_conf.c +++ libvirt-acl/src/qemu/qemu_conf.c @@ -1465,9 +1465,8 @@ int qemudPhysIfaceConnect(virConnectPtr conn, struct qemud_driver *driver, virDomainNetDefPtr net, - char *linkdev, - int brmode, - unsigned long long qemuCmdFlags) + unsigned long long qemuCmdFlags, + const unsigned char *vmuuid) { int rc; #if WITH_MACVTAP @@ -1479,8 +1478,7 @@ qemudPhysIfaceConnect(virConnectPtr conn net->model && STREQ(net->model, "virtio")) vnet_hdr = 1; - rc = openMacvtapTap(net->ifname, net->mac, linkdev, brmode, - &res_ifname, vnet_hdr); + rc = openMacvtapTap(net->ifname, net, &res_ifname, vnet_hdr, vmuuid); if (rc >= 0) { VIR_FREE(net->ifname); net->ifname = res_ifname; @@ -1500,15 +1498,13 @@ qemudPhysIfaceConnect(virConnectPtr conn if (err) { close(rc); rc = -1; - delMacvtap(net->ifname); + delMacvtap(net); } } } #else (void)conn; (void)net; - (void)linkdev; - (void)brmode; (void)qemuCmdFlags; (void)driver; qemuReportError(VIR_ERR_INTERNAL_ERROR, @@ -4130,9 +4126,8 @@ int qemudBuildCommandLine(virConnectPtr goto no_memory; } else if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) { int tapfd = qemudPhysIfaceConnect(conn, driver, net, - net->data.direct.linkdev, - net->data.direct.mode, - qemuCmdFlags); + qemuCmdFlags, + def->uuid); if (tapfd < 0) goto error;

1 0

[libvirt] [PATCH] Fix handling of disk backing stores with cgroups
by Daniel P. Berrange 13 May '10

13 May '10

The cgroups ACL code was only allowing the primary disk image. It is possible to chain images together, so we need to search for backing stores and add them to the ACL too. Since the ACL only handles block devices, we ignore the EINVAL we get from plain files. In addition it was missing code to teardown the cgroup when hot-unplugging a disk * src/qemu/qemu_driver.c: Allow backing stores in cgroup ACLs and add missing teardown code in unplug path --- src/qemu/qemu_driver.c | 170 +++++++++++++++++++++++++++++++++++++++--------- 1 files changed, 139 insertions(+), 31 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 92dc22d..f615072 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2929,6 +2929,102 @@ static const char *const defaultDeviceACL[] = { #define DEVICE_PTY_MAJOR 136 #define DEVICE_SND_MAJOR 116 +static int qemuSetupDiskCgroup(virCgroupPtr cgroup, + virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ + char *path = disk->src; + int ret = -1; + + while (path != NULL) { + virStorageFileMetadata meta; + int rc; + + VIR_DEBUG("Process path %s for disk", path); + rc = virCgroupAllowDevicePath(cgroup, path); + if (rc != 0) { + /* Get this for non-block devices */ + if (rc == -EINVAL) { + VIR_DEBUG("Ignoring EINVAL for %s", path); + } else { + virReportSystemError(-rc, + _("Unable to allow device %s for %s"), + path, vm->def->name); + if (path != disk->src) + VIR_FREE(path); + goto cleanup; + } + } + + memset(&meta, 0, sizeof(meta)); + + rc = virStorageFileGetMetadata(path, &meta); + + if (path != disk->src) + VIR_FREE(path); + path = NULL; + + if (rc < 0) + goto cleanup; + + path = meta.backingStore; + } while (path != NULL); + + ret = 0; + +cleanup: + return ret; +} + + +static int qemuTeardownDiskCgroup(virCgroupPtr cgroup, + virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ + char *path = disk->src; + int ret = -1; + + while (path != NULL) { + virStorageFileMetadata meta; + int rc; + + VIR_DEBUG("Process path %s for disk", path); + rc = virCgroupDenyDevicePath(cgroup, path); + if (rc != 0) { + /* Get this for non-block devices */ + if (rc == -EINVAL) { + VIR_DEBUG("Ignoring EINVAL for %s", path); + } else { + virReportSystemError(-rc, + _("Unable to deny device %s for %s"), + path, vm->def->name); + if (path != disk->src) + VIR_FREE(path); + goto cleanup; + } + } + + memset(&meta, 0, sizeof(meta)); + + rc = virStorageFileGetMetadata(path, &meta); + + if (path != disk->src) + VIR_FREE(path); + path = NULL; + + if (rc < 0) + goto cleanup; + + path = meta.backingStore; + } while (path != NULL); + + ret = 0; + +cleanup: + return ret; +} + + static int qemuSetupCgroup(struct qemud_driver *driver, virDomainObjPtr vm) { @@ -2965,18 +3061,8 @@ static int qemuSetupCgroup(struct qemud_driver *driver, } for (i = 0; i < vm->def->ndisks ; i++) { - if (vm->def->disks[i]->type != VIR_DOMAIN_DISK_TYPE_BLOCK || - vm->def->disks[i]->src == NULL) - continue; - - rc = virCgroupAllowDevicePath(cgroup, - vm->def->disks[i]->src); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to allow device %s for %s"), - vm->def->disks[i]->src, vm->def->name); + if (qemuSetupDiskCgroup(cgroup, vm, vm->def->disks[i]) < 0) goto cleanup; - } } rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR); @@ -7550,15 +7636,8 @@ static int qemudDomainAttachDevice(virDomainPtr dom, vm->def->name); goto endjob; } - if (dev->data.disk->src != NULL && - dev->data.disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK && - virCgroupAllowDevicePath(cgroup, - dev->data.disk->src) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("unable to allow device %s"), - dev->data.disk->src); + if (qemuSetupDiskCgroup(cgroup, vm, dev->data.disk) < 0) goto endjob; - } } switch (dev->data.disk->device) { @@ -7602,8 +7681,9 @@ static int qemudDomainAttachDevice(virDomainPtr dom, /* Fallthrough */ } if (ret != 0 && cgroup) { - virCgroupDenyDevicePath(cgroup, - dev->data.disk->src); + if (qemuTeardownDiskCgroup(cgroup, vm, dev->data.disk) < 0) + VIR_WARN("Failed to teardown cgroup for disk path %s", + NULLSTR(dev->data.disk->src)); } } else if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { if (dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { @@ -7801,15 +7881,8 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, vm->def->name); goto endjob; } - if (dev->data.disk->src != NULL && - dev->data.disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK && - virCgroupAllowDevicePath(cgroup, - dev->data.disk->src) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("unable to allow device %s"), - dev->data.disk->src); + if (qemuSetupDiskCgroup(cgroup, vm, dev->data.disk) < 0) goto endjob; - } } switch (dev->data.disk->device) { @@ -7831,8 +7904,9 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, } if (ret != 0 && cgroup) { - virCgroupDenyDevicePath(cgroup, - dev->data.disk->src); + if (qemuTeardownDiskCgroup(cgroup, vm, dev->data.disk) < 0) + VIR_WARN("Failed to teardown cgroup for disk path %s", + NULLSTR(dev->data.disk->src)); } break; @@ -7904,6 +7978,7 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, int i, ret = -1; virDomainDiskDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; + virCgroupPtr cgroup = NULL; i = qemudFindDisk(vm->def, dev->data.disk->dst); @@ -7915,6 +7990,15 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, detach = vm->def->disks[i]; + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { + if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to find cgroup for %s\n"), + vm->def->name); + goto cleanup; + } + } + if (!virDomainDeviceAddressIsValid(&detach->info, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", @@ -7939,11 +8023,19 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, qemudShrinkDisks(vm->def, i); + virDomainDiskDefFree(detach); + if (driver->securityDriver && driver->securityDriver->domainRestoreSecurityImageLabel && driver->securityDriver->domainRestoreSecurityImageLabel(vm, dev->data.disk) < 0) VIR_WARN("Unable to restore security label on %s", dev->data.disk->src); + if (cgroup != NULL) { + if (qemuTeardownDiskCgroup(cgroup, vm, dev->data.disk) < 0) + VIR_WARN("Failed to teardown cgroup for disk path %s", + NULLSTR(dev->data.disk->src)); + } + ret = 0; cleanup: @@ -7958,6 +8050,7 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, int i, ret = -1; virDomainDiskDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; + virCgroupPtr cgroup = NULL; i = qemudFindDisk(vm->def, dev->data.disk->dst); @@ -7975,6 +8068,15 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, detach = vm->def->disks[i]; + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { + if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to find cgroup for %s\n"), + vm->def->name); + goto cleanup; + } + } + qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { qemuDomainObjExitMonitor(vm); @@ -7991,6 +8093,12 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, driver->securityDriver->domainRestoreSecurityImageLabel(vm, dev->data.disk) < 0) VIR_WARN("Unable to restore security label on %s", dev->data.disk->src); + if (cgroup != NULL) { + if (qemuTeardownDiskCgroup(cgroup, vm, dev->data.disk) < 0) + VIR_WARN("Failed to teardown cgroup for disk path %s", + NULLSTR(dev->data.disk->src)); + } + ret = 0; cleanup: -- 1.6.5.2

2 1

[libvirt] [PATCHv2 0/10+] error message cleanup
by Eric Blake 13 May '10

13 May '10

I've done some more work on error cleanup, expanding 3 patches of v1 into 10 patches, with more still to be written. See individual patches for more comments. bootstrap.conf | 1 + cfg.mk | 5 +- src/datatypes.c | 127 +++++++++++---- src/libvirt.c | 94 ++++++----- src/security/virt-aa-helper.c | 141 ++++++++-------- src/storage/storage_backend.c | 2 +- src/util/virterror.c | 374 +++++++++++++--------------------------- src/xen/xend_internal.c | 5 +- tests/cpuset | 4 +- tests/undefine | 4 +- 10 files changed, 352 insertions(+), 405 deletions(-) Net reduction in lines of code is a good thing. [PATCHv2 01/10] virt-aa-helper: translate error messages [PATCHv2 02/10] build: use gnulib func module [PATCHv2 03/10] datatypes: avoid redundant __FUNCTION__ [PATCHv2 04/10] virErrorMsg: refactor to simplify common case [PATCHv2 05/10] virterror: start messages with lower case [PATCHv2 06/10] virterror: clean up VIR_ERR_HTTP_ERROR usage [PATCHv2 07/10] virterror: clean up VIR_ERR_XML_DETAIL usage [PATCHv2 08/10] virterror: clean up several seldom-used errors [PATCHv2 09/10] virterror: clean up VIR_ERR_INVALID_NETWORK usage [PATCHv2 10/10] virterror: clean up VIR_ERR_INVALID_INTERFACE usage [PATCHv2 N/N] to be written, after feedback on this round

1 10