- Devel - libvirt List Archives

[libvirt] [PATCH] qemu: Avoid leak in qemuDomainCheckRemoveOptionalDisk
by Jiri Denemark 15 May '14

15 May '14

Coverity complains about event being leaked in qemuDomainCheckRemoveOptionalDisk. The best fix for it is to avoid searching for the disk (using its srouce path even) we got by walking through vm->def->disks array. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/qemu/qemu_domain.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7a0be12..78cfdc6 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2176,11 +2176,11 @@ qemuDomainSetFakeReboot(virQEMUDriverPtr driver, static int qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainDiskDefPtr disk) + size_t diskIndex) { char uuid[VIR_UUID_STRING_BUFLEN]; virObjectEventPtr event = NULL; - virDomainDiskDefPtr del_disk = NULL; + virDomainDiskDefPtr disk = vm->def->disks[diskIndex]; const char *src = virDomainDiskGetSource(disk); virUUIDFormat(vm->def->uuid, uuid); @@ -2200,13 +2200,8 @@ qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr driver, event = virDomainEventDiskChangeNewFromObj(vm, src, NULL, disk->info.alias, VIR_DOMAIN_EVENT_DISK_DROP_MISSING_ON_START); - - if (!(del_disk = virDomainDiskRemoveByName(vm->def, src))) { - virReportError(VIR_ERR_INVALID_ARG, - _("no source device %s"), src); - return -1; - } - virDomainDiskDefFree(del_disk); + virDomainDiskRemove(vm->def, diskIndex); + virDomainDiskDefFree(disk); } if (event) @@ -2218,11 +2213,11 @@ qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr driver, static int qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainDiskDefPtr disk, + size_t diskIndex, bool cold_boot) { char uuid[VIR_UUID_STRING_BUFLEN]; - int startupPolicy = disk->startupPolicy; + int startupPolicy = vm->def->disks[diskIndex]->startupPolicy; virUUIDFormat(vm->def->uuid, uuid); @@ -2244,7 +2239,7 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver, break; } - if (qemuDomainCheckRemoveOptionalDisk(driver, vm, disk) < 0) + if (qemuDomainCheckRemoveOptionalDisk(driver, vm, diskIndex) < 0) goto error; return 0; @@ -2282,11 +2277,11 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, { int ret = -1; size_t i; - virDomainDiskDefPtr disk; VIR_DEBUG("Checking for disk presence"); for (i = vm->def->ndisks; i > 0; i--) { - disk = vm->def->disks[i - 1]; + size_t idx = i - 1; + virDomainDiskDefPtr disk = vm->def->disks[idx]; const char *path = virDomainDiskGetSource(disk); virStorageFileFormat format = virDomainDiskGetFormat(disk); virStorageType type = virStorageSourceGetActualType(&disk->src); @@ -2308,7 +2303,7 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, continue; if (disk->startupPolicy && - qemuDomainCheckDiskStartupPolicy(driver, vm, disk, + qemuDomainCheckDiskStartupPolicy(driver, vm, idx, cold_boot) >= 0) { virResetLastError(); continue; -- 1.9.3

5 5

[libvirt] [PATCH] virsh: For virsh migrate --tunnelled, use the virDomainMigrateToURI3 API.
by Richard W.M. Jones 15 May '14

15 May '14

Using virsh migrate + the --tunnelled flag causes virsh to use the wrong API. This gives the error: error: use virDomainMigrateToURI3 for peer-to-peer migration As the error message is wrong, this patch also corrects the error message. https://bugzilla.redhat.com/show_bug.cgi?id=1095924 Signed-off-by: Richard W.M. Jones <rjones(a)redhat.com> Cc: Jiri Denemark <jdenemar(a)redhat.com> --- src/libvirt.c | 2 +- tools/virsh-domain.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libvirt.c b/src/libvirt.c index 2cd793c..6a361f6 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -5726,7 +5726,7 @@ virDomainMigrate3(virDomainPtr domain, if (flags & (VIR_MIGRATE_PEER2PEER | VIR_MIGRATE_TUNNELLED)) { virReportInvalidArg(flags, "%s", _("use virDomainMigrateToURI3 for peer-to-peer " - "migration")); + "or tunnelled migration")); goto error; } diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 3a7c260..5a5ce9d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -8959,6 +8959,7 @@ doMigrate(void *opaque) flags |= VIR_MIGRATE_ABORT_ON_ERROR; if ((flags & VIR_MIGRATE_PEER2PEER) || + (flags & VIR_MIGRATE_TUNNELLED) || vshCommandOptBool(cmd, "direct")) { /* migrateuri doesn't make sense for tunnelled migration */ -- 1.9.0

2 2

[libvirt] [PATCH 0/3] security_dac: trivial cleanups
by Jim Fehlig 14 May '14

14 May '14

A while back, Michal Privoznik posted a small series to honor the DAC norelabel attribute https://www.redhat.com/archives/libvir-list/2014-April/msg00196.html When reviewing patch 1, Jan Tomko noted that trivial whitespace changes and the like should be separate from changes to the callback parameter passing. This small series contains the most trivial changes Michal made to patch 1. By "most trivial", I mean these could probably be pushed under the trivail rule, but posting for a quick review nonetheless. Jim Fehlig (3): security_dac: Remove unnecessary ATTRIBUTE_UNUSED security_dac: Remove unnecessary curly braces security_dac: Fix indentation src/security/security_dac.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) -- 1.8.1.4

2 6

[libvirt] [PATCH] virsh: Return false if only '--wipe-storage' is assigned when undefine a domain
by Li Yang 14 May '14

14 May '14

For now, if only '--wipe-storage' is assigned, user can undefine a domain normally. But actually '--wipe-storage' doesn't work, this may confuse user. And since '--wipe-storage' wipes data on the removed volumes, if no removed volume storage assigned, we'd better raise an error message. Before: $ virsh undefine virt-tests-vm1 --wipe-storage Domain virt-tests-vm1 has been undefined After: $ virsh undefine virt-tests-vm1 --wipe-storage error: '--wipe-storage' needs storage volume deletion: '--stroage <string>' or '--remove-all-storage' is necessary. Signed-off-by: Li Yang <liyang.fnst(a)cn.fujitsu.com> --- tools/virsh-domain.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 3a7c260..25236a0 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2982,6 +2982,14 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) ignore_value(vshCommandOptString(cmd, "storage", &vol_string)); + if (!(vol_string || remove_all_storage) && wipe_storage) { + vshError(ctl, + _("'--wipe-storage' needs storage volume deletion: " + "'--stroage <string>' or '--remove-all-storage' " + "is necessary.")); + return false; + } + if (managed_save) { flags |= VIR_DOMAIN_UNDEFINE_MANAGED_SAVE; managed_save_safe = true; -- 1.7.1

2 1

[libvirt] [PATCH 1/4] conf: use typedefs for enums in "src/conf/{node_device_conf, nwfilter_params}.h"
by Julio Faracco 14 May '14

14 May '14

In "src/conf/" there are many enumeration (enum) declarations. Similar to the recent cleanup to "src/util" directory, it's better to use a typedef for variable types, function types and other usages. Other enumeration and folders will be changed to typedef's in the future. Most of the files changed in this commit are reltaed to Network (node_device_conf.h and nwfilter_params.*) enums. Signed-off-by: Julio Faracco <jcfaracco(a)gmail.com> Signed-off-by: Eric Blake <eblake(a)redhat.com> --- src/conf/node_device_conf.h | 24 ++++++++++++------------ src/conf/nwfilter_params.c | 2 +- src/conf/nwfilter_params.h | 14 +++++++------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 9b5d172..50e6805 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -35,7 +35,7 @@ # define CREATE_DEVICE 1 # define EXISTING_DEVICE 0 -enum virNodeDevCapType { +typedef enum { /* Keep in sync with VIR_ENUM_IMPL in node_device_conf.c */ VIR_NODE_DEV_CAP_SYSTEM, /* System capability */ VIR_NODE_DEV_CAP_PCI_DEV, /* PCI device */ @@ -51,38 +51,38 @@ enum virNodeDevCapType { VIR_NODE_DEV_CAP_SCSI_GENERIC, /* SCSI generic device */ VIR_NODE_DEV_CAP_LAST -}; +} virNodeDevCapType; -enum virNodeDevNetCapType { +typedef enum { /* Keep in sync with VIR_ENUM_IMPL in node_device_conf.c */ VIR_NODE_DEV_CAP_NET_80203, /* 802.03 network device */ VIR_NODE_DEV_CAP_NET_80211, /* 802.11 network device */ VIR_NODE_DEV_CAP_NET_LAST -}; +} virNodeDevNetCapType; VIR_ENUM_DECL(virNodeDevCap) VIR_ENUM_DECL(virNodeDevNetCap) -enum virNodeDevStorageCapFlags { +typedef enum { VIR_NODE_DEV_CAP_STORAGE_REMOVABLE = (1 << 0), VIR_NODE_DEV_CAP_STORAGE_REMOVABLE_MEDIA_AVAILABLE = (1 << 1), VIR_NODE_DEV_CAP_STORAGE_HOTPLUGGABLE = (1 << 2), -}; +} virNodeDevStorageCapFlags; -enum virNodeDevSCSIHostCapFlags { +typedef enum { VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST = (1 << 0), VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS = (1 << 1), -}; +} virNodeDevSCSIHostCapFlags; -enum virNodeDevPCICapFlags { +typedef enum { VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION = (1 << 0), VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION = (1 << 1), -}; +} virNodeDevPCICapFlags; typedef struct _virNodeDevCapsDef virNodeDevCapsDef; typedef virNodeDevCapsDef *virNodeDevCapsDefPtr; struct _virNodeDevCapsDef { - enum virNodeDevCapType type; + virNodeDevCapType type; union _virNodeDevCapData { struct { char *product_name; @@ -135,7 +135,7 @@ struct _virNodeDevCapsDef { char *address; unsigned int address_len; char *ifname; - enum virNodeDevNetCapType subtype; /* LAST -> no subtype */ + virNodeDevNetCapType subtype; /* LAST -> no subtype */ } net; struct { unsigned int host; diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c index ac4d4a8..a12e645 100644 --- a/src/conf/nwfilter_params.c +++ b/src/conf/nwfilter_params.c @@ -1061,7 +1061,7 @@ virNWFilterVarAccessGetVarName(const virNWFilterVarAccess *vap) return vap->varName; } -enum virNWFilterVarAccessType +virNWFilterVarAccessType virNWFilterVarAccessGetType(const virNWFilterVarAccess *vap) { return vap->accessType; diff --git a/src/conf/nwfilter_params.h b/src/conf/nwfilter_params.h index 08e448f..98610a7 100644 --- a/src/conf/nwfilter_params.h +++ b/src/conf/nwfilter_params.h @@ -27,17 +27,17 @@ # include "virbuffer.h" # include "virmacaddr.h" -enum virNWFilterVarValueType { +typedef enum { NWFILTER_VALUE_TYPE_SIMPLE, NWFILTER_VALUE_TYPE_ARRAY, NWFILTER_VALUE_TYPE_LAST -}; +} virNWFilterVarValueType; typedef struct _virNWFilterVarValue virNWFilterVarValue; typedef virNWFilterVarValue *virNWFilterVarValuePtr; struct _virNWFilterVarValue { - enum virNWFilterVarValueType valType; + virNWFilterVarValueType valType; union { struct { char *value; @@ -98,17 +98,17 @@ bool virNWFilterHashTableEqual(virNWFilterHashTablePtr a, # define NWFILTER_VARNAME_CTRL_IP_LEARNING "CTRL_IP_LEARNING" # define NWFILTER_VARNAME_DHCPSERVER "DHCPSERVER" -enum virNWFilterVarAccessType { +typedef enum { VIR_NWFILTER_VAR_ACCESS_ELEMENT = 0, VIR_NWFILTER_VAR_ACCESS_ITERATOR = 1, VIR_NWFILTER_VAR_ACCESS_LAST, -}; +} virNWFilterVarAccessType; typedef struct _virNWFilterVarAccess virNWFilterVarAccess; typedef virNWFilterVarAccess *virNWFilterVarAccessPtr; struct _virNWFilterVarAccess { - enum virNWFilterVarAccessType accessType; + virNWFilterVarAccessType accessType; union { struct { unsigned int index; @@ -128,7 +128,7 @@ virNWFilterVarAccessPtr virNWFilterVarAccessParse(const char *varAccess); void virNWFilterVarAccessPrint(virNWFilterVarAccessPtr vap, virBufferPtr buf); const char *virNWFilterVarAccessGetVarName(const virNWFilterVarAccess *vap); -enum virNWFilterVarAccessType virNWFilterVarAccessGetType( +virNWFilterVarAccessType virNWFilterVarAccessGetType( const virNWFilterVarAccess *vap); unsigned int virNWFilterVarAccessGetIterId(const virNWFilterVarAccess *vap); unsigned int virNWFilterVarAccessGetIndex(const virNWFilterVarAccess *vap); -- 1.7.10.4

4 12

[libvirt] submitted patch, don't see in pending.
by vikhyath reddy 14 May '14

14 May '14

Hi all, I had a question regarding submitting patches for libvirt. I submitted a hyperv patch about two weeks back using the tips mentioned in http://libvirt.org/hacking.html#patches using git send-email But I dont see the patch in the pending list at http://libvirt.org/pending.html (Is there something I missed or something failed to work as expected) OR (is the pending list not real time)? Thanks guys for all your help! Vik.

2 2

[libvirt] [PATCH 0/3] qemu: Ignore temporary job errors when checking migration status
by Jiri Denemark 14 May '14

14 May '14

https://bugzilla.redhat.com/show_bug.cgi?id=1083238 Jiri Denemark (3): qemu: Make qemuDomainObjBeginNestedJob static qemuDomainObjBeginNestedJob: Return -2 for temporary failures qemu: Ignore temporary job errors when checking migration status src/qemu/qemu_domain.c | 41 +++++++++++++++++++++++++++-------------- src/qemu/qemu_domain.h | 4 ---- src/qemu/qemu_migration.c | 7 ++++--- 3 files changed, 31 insertions(+), 21 deletions(-) -- 1.9.3

2 10

[libvirt] [PATCH] Don't log an internal error when the guest hasn't updated balloon stats
by Ján Tomko 14 May '14

14 May '14

If virDomainMemoryStats is called too soon after domain startup, QEMU returns: "error":{"class":"GenericError","desc":"guest hasn't updated any stats yet"} when we try to query balloon stats. Check for this reply and log it as OPERATION_INVALID instead of INTERNAL_ERROR. This means the daemon only logs it at the debug level, without polluting system logs. Reported by Laszlo Pal: https://www.redhat.com/archives/libvirt-users/2014-May/msg00023.html --- src/qemu/qemu_monitor_json.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index f8ab975..88f6b6c 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1460,26 +1460,38 @@ int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon, goto cleanup; if (!(cmd = qemuMonitorJSONMakeCommand("qom-get", "s:path", balloonpath, "s:property", "guest-stats", NULL))) goto cleanup; ret = qemuMonitorJSONCommand(mon, cmd, &reply); - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - if (ret < 0) goto cleanup; + if ((data = virJSONValueObjectGet(reply, "error"))) { + const char *klass = virJSONValueObjectGetString(data, "class"); + const char *desc = virJSONValueObjectGetString(data, "desc"); + + if (STREQ_NULLABLE(klass, "GenericError") && + STREQ_NULLABLE(desc, "guest hasn't updated any stats yet")) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("the guest hasn't updated any stats yet")); + ret = -1; + goto cleanup; + } + } + + ret = qemuMonitorJSONCheckError(cmd, reply); + if (!(data = virJSONValueObjectGet(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("qom-get reply was missing return data")); goto cleanup; } if (!(statsdata = virJSONValueObjectGet(data, "stats"))) { VIR_DEBUG("data does not include 'stats'"); goto cleanup; } -- 1.8.3.2

2 1

[libvirt] [PATCH] Prevent guest be paused when do external system checkpoint snapshot
by Shanzhi Yu 14 May '14

14 May '14

https://bugzilla.redhat.com/show_bug.cgi?id=1097503 Guest will be paused when do external system checkpoint snapshot with invalid compression format. Signed-off-by: Shanzhi Yu <shyu(a)redhat.com> --- src/qemu/qemu_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a5c7f4f..0761760 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13207,13 +13207,13 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn, virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Invalid snapshot image format specified " "in configuration file")); - goto cleanup; + goto endjob; } if (!qemuCompressProgramAvailable(compressed)) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Compression program for image format " "in configuration file isn't available")); - goto cleanup; + goto endjob; } } -- 1.8.3.1

2 1

[libvirt] [PATCH] network: Defer online of macvtap during qemu migration
by Matthew Rosato 14 May '14

14 May '14

When generating macvtaps via virNetDevMacVLanCreateWithVPortProfile, the macvtap device is unconditionally set to the up state. However, during migration, this results in a case where both the source and target system are simultaneously up with the same MAC address. This patch defers bringing the target macvtap up until later in the migration to shrink this window. Signed-off-by: Matthew Rosato <mjrosato(a)linux.vnet.ibm.com> --- src/qemu/qemu_migration.c | 18 ++++++++++++++++++ src/util/virnetdevmacvlan.c | 11 ++++++++--- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index a9f7fea..aee803a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -56,6 +56,7 @@ #include "virhook.h" #include "virstring.h" #include "virtypedparam.h" +#include "virnetdev.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -4468,6 +4469,8 @@ qemuMigrationFinish(virQEMUDriverPtr driver, virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virCapsPtr caps = NULL; unsigned short port; + virDomainNetDefPtr net; + size_t i; VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, flags=%lx, retcode=%d", @@ -4574,6 +4577,21 @@ qemuMigrationFinish(virQEMUDriverPtr driver, } if (!(flags & VIR_MIGRATE_PAUSED) && !(flags & VIR_MIGRATE_OFFLINE)) { + /* Macvtaps were previously left offline, bring them online now */ + for (i = 0; i < vm->def->nnets; i++) { + net = vm->def->nets[i]; + if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT) { + if (virNetDevSetOnline(net->ifname, true) < 0) { + ignore_value(virNetDevVPortProfileDisassociate(net->ifname, + virDomainNetGetActualVirtPortProfile(net), + &net->mac, + virDomainNetGetActualDirectDev(net), + -1, + VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH)); + ignore_value(virNetDevMacVLanDelete(net->ifname)); + } + } + } /* run 'cont' on the destination, which allows migration on qemu * >= 0.10.6 to work properly. This isn't strictly necessary on * older qemu's, but it also doesn't hurt anything there diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index 7bbf540..3da845b 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -898,9 +898,14 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname, goto link_del_exit; } - if (virNetDevSetOnline(cr_ifname, true) < 0) { - rc = -1; - goto disassociate_exit; + /* If this device is being created as part of an inbound + * migration, leave the device offline for now. + */ + if (vmOp != VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START) { + if (virNetDevSetOnline(cr_ifname, true) < 0) { + rc = -1; + goto disassociate_exit; + } } if (withTap) { -- 1.7.9.5

3 5