- Devel - libvirt List Archives

[libvirt] [PATCH] libvirt supports Guest Panicked
by Chen Fan 28 May '13

28 May '13

From: chenfan <chen.fan.fnst(a)cn.fujitsu.com> This patch implements qemu_driver supporting guest panicked, modified the 'on_crash' default value to 'preserve'. --- examples/domain-events/events-c/event-test.c | 10 +++ include/libvirt/libvirt.h.in | 16 +++++ src/conf/domain_conf.c | 14 ++-- src/qemu/qemu_monitor.c | 14 +++- src/qemu/qemu_monitor.h | 5 ++ src/qemu/qemu_monitor_json.c | 7 ++ src/qemu/qemu_process.c | 99 +++++++++++++++++++++++++++- tools/virsh-domain-monitor.c | 8 +++ 8 files changed, 166 insertions(+), 7 deletions(-) diff --git a/examples/domain-events/events-c/event-test.c b/examples/domain-events/events-c/event-test.c index eeff50f..1b425fb 100644 --- a/examples/domain-events/events-c/event-test.c +++ b/examples/domain-events/events-c/event-test.c @@ -93,6 +93,9 @@ const char *eventToString(int event) { case VIR_DOMAIN_EVENT_PMSUSPENDED: ret = "PMSuspended"; break; + case VIR_DOMAIN_EVENT_CRASHED: + ret = "Crashed"; + break; } return ret; } @@ -209,6 +212,13 @@ static const char *eventDetailToString(int event, int detail) { break; } break; + case VIR_DOMAIN_EVENT_CRASHED: + switch ((virDomainEventCrashedDetailType) detail) { + case VIR_DOMAIN_EVENT_CRASHED_PANICKED: + ret = "Panicked"; + break; + } + break; } return ret; } diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 1804c93..56c6c5c 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -155,6 +155,7 @@ typedef enum { VIR_DOMAIN_RUNNING_SAVE_CANCELED = 7, /* returned from failed save process */ VIR_DOMAIN_RUNNING_WAKEUP = 8, /* returned from pmsuspended due to wakeup event */ + VIR_DOMAIN_RUNNING_CRASHED = 9, /* resumed from crashed */ #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_RUNNING_LAST @@ -180,6 +181,7 @@ typedef enum { VIR_DOMAIN_PAUSED_FROM_SNAPSHOT = 7, /* paused after restoring from snapshot */ VIR_DOMAIN_PAUSED_SHUTTING_DOWN = 8, /* paused during shutdown process */ VIR_DOMAIN_PAUSED_SNAPSHOT = 9, /* paused while creating a snapshot */ + VIR_DOMAIN_PAUSED_GUEST_PANICKED = 10, /* paused due to a guest panicked event */ #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_PAUSED_LAST @@ -189,6 +191,7 @@ typedef enum { typedef enum { VIR_DOMAIN_SHUTDOWN_UNKNOWN = 0, /* the reason is unknown */ VIR_DOMAIN_SHUTDOWN_USER = 1, /* shutting down on user request */ + VIR_DOMAIN_SHUTDOWN_CRASHED = 2, /* domain crashed */ #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_SHUTDOWN_LAST @@ -212,6 +215,7 @@ typedef enum { typedef enum { VIR_DOMAIN_CRASHED_UNKNOWN = 0, /* crashed for unknown reason */ + VIR_DOMAIN_CRASHED_PANICKED = 1, /* domain panicked */ #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_CRASHED_LAST @@ -3319,6 +3323,7 @@ typedef enum { VIR_DOMAIN_EVENT_STOPPED = 5, VIR_DOMAIN_EVENT_SHUTDOWN = 6, VIR_DOMAIN_EVENT_PMSUSPENDED = 7, + VIR_DOMAIN_EVENT_CRASHED = 8, #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_EVENT_LAST @@ -3450,6 +3455,17 @@ typedef enum { #endif } virDomainEventPMSuspendedDetailType; +/* + * Details about the 'crashed' lifecycle event + */ +typedef enum { + VIR_DOMAIN_EVENT_CRASHED_PANICKED = 0, /* Guest was panicked */ + +#ifdef VIR_ENUM_SENTINELS + VIR_DOMAIN_EVENT_CRASHED_LAST +#endif +} virDomainEventCrashedDetailType; + /** * virConnectDomainEventCallback: * @conn: virConnect connection diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a9656af..3f0786e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -642,7 +642,8 @@ VIR_ENUM_IMPL(virDomainRunningReason, VIR_DOMAIN_RUNNING_LAST, "unpaused", "migration canceled", "save canceled", - "wakeup") + "wakeup", + "from crashed") VIR_ENUM_IMPL(virDomainBlockedReason, VIR_DOMAIN_BLOCKED_LAST, "unknown") @@ -657,11 +658,13 @@ VIR_ENUM_IMPL(virDomainPausedReason, VIR_DOMAIN_PAUSED_LAST, "watchdog", "from snapshot", "shutdown", - "snapshot") + "snapshot", + "guest panicked") VIR_ENUM_IMPL(virDomainShutdownReason, VIR_DOMAIN_SHUTDOWN_LAST, "unknown", - "user") + "user", + "crashed") VIR_ENUM_IMPL(virDomainShutoffReason, VIR_DOMAIN_SHUTOFF_LAST, "unknown", @@ -674,7 +677,8 @@ VIR_ENUM_IMPL(virDomainShutoffReason, VIR_DOMAIN_SHUTOFF_LAST, "from snapshot") VIR_ENUM_IMPL(virDomainCrashedReason, VIR_DOMAIN_CRASHED_LAST, - "unknown") + "unknown", + "panicked") VIR_ENUM_IMPL(virDomainPMSuspendedReason, VIR_DOMAIN_PMSUSPENDED_LAST, "unknown") @@ -10943,7 +10947,7 @@ virDomainDefParseXML(xmlDocPtr xml, if (virDomainEventActionParseXML(ctxt, "on_crash", "string(./on_crash[1])", &def->onCrash, - VIR_DOMAIN_LIFECYCLE_CRASH_DESTROY, + VIR_DOMAIN_LIFECYCLE_CRASH_PRESERVE, virDomainLifecycleCrashTypeFromString) < 0) goto error; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 4e35f79..e0cd62c 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -113,7 +113,7 @@ VIR_ENUM_IMPL(qemuMonitorVMStatus, QEMU_MONITOR_VM_STATUS_LAST, "debug", "inmigrate", "internal-error", "io-error", "paused", "postmigrate", "prelaunch", "finish-migrate", "restore-vm", - "running", "save-vm", "shutdown", "watchdog") + "running", "save-vm", "shutdown", "watchdog", "guest-panicked") typedef enum { QEMU_MONITOR_BLOCK_IO_STATUS_OK, @@ -1032,6 +1032,15 @@ int qemuMonitorEmitResume(qemuMonitorPtr mon) } +int qemuMonitorEmitGUESTPanicked(qemuMonitorPtr mon) +{ + int ret = -1; + VIR_DEBUG("mon=%p", mon); + QEMU_MONITOR_CALLBACK(mon, ret, domainGUESTPanicked, mon->vm); + return ret; +} + + int qemuMonitorEmitRTCChange(qemuMonitorPtr mon, long long offset) { int ret = -1; @@ -3185,6 +3194,9 @@ int qemuMonitorVMStatusToPausedReason(const char *status) case QEMU_MONITOR_VM_STATUS_WATCHDOG: return VIR_DOMAIN_PAUSED_WATCHDOG; + case QEMU_MONITOR_VM_STATUS_GUEST_PANICKED: + return VIR_DOMAIN_PAUSED_GUEST_PANICKED; + /* unreachable from this point on */ case QEMU_MONITOR_VM_STATUS_LAST: ; diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index a607712..543050c 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -140,6 +140,8 @@ struct _qemuMonitorCallbacks { unsigned long long actual); int (*domainPMSuspendDisk)(qemuMonitorPtr mon, virDomainObjPtr vm); + int (*domainGUESTPanicked)(qemuMonitorPtr mon, + virDomainObjPtr vm); }; char *qemuMonitorEscapeArg(const char *in); @@ -220,6 +222,8 @@ int qemuMonitorEmitBlockJob(qemuMonitorPtr mon, int qemuMonitorEmitBalloonChange(qemuMonitorPtr mon, unsigned long long actual); int qemuMonitorEmitPMSuspendDisk(qemuMonitorPtr mon); +int qemuMonitorEmitGUESTPanicked(qemuMonitorPtr mon); + int qemuMonitorStartCPUs(qemuMonitorPtr mon, virConnectPtr conn); @@ -239,6 +243,7 @@ typedef enum { QEMU_MONITOR_VM_STATUS_SAVE_VM, QEMU_MONITOR_VM_STATUS_SHUTDOWN, QEMU_MONITOR_VM_STATUS_WATCHDOG, + QEMU_MONITOR_VM_STATUS_GUEST_PANICKED, QEMU_MONITOR_VM_STATUS_LAST } qemuMonitorVMStatus; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 2b73884..b6efa52 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -74,6 +74,7 @@ static void qemuMonitorJSONHandleBlockJobCanceled(qemuMonitorPtr mon, virJSONVal static void qemuMonitorJSONHandleBlockJobReady(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleBalloonChange(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandlePMSuspendDisk(qemuMonitorPtr mon, virJSONValuePtr data); +static void qemuMonitorJSONHandleGUESTPanicked(qemuMonitorPtr mon, virJSONValuePtr data); typedef struct { const char *type; @@ -87,6 +88,7 @@ static qemuEventHandler eventHandlers[] = { { "BLOCK_JOB_COMPLETED", qemuMonitorJSONHandleBlockJobCompleted, }, { "BLOCK_JOB_READY", qemuMonitorJSONHandleBlockJobReady, }, { "DEVICE_TRAY_MOVED", qemuMonitorJSONHandleTrayChange, }, + { "GUEST_PANICKED", qemuMonitorJSONHandleGUESTPanicked, }, { "POWERDOWN", qemuMonitorJSONHandlePowerdown, }, { "RESET", qemuMonitorJSONHandleReset, }, { "RESUME", qemuMonitorJSONHandleResume, }, @@ -593,6 +595,11 @@ static void qemuMonitorJSONHandleResume(qemuMonitorPtr mon, virJSONValuePtr data qemuMonitorEmitResume(mon); } +static void qemuMonitorJSONHandleGUESTPanicked(qemuMonitorPtr mon, virJSONValuePtr data ATTRIBUTE_UNUSED) +{ + qemuMonitorEmitGUESTPanicked(mon); +} + static void qemuMonitorJSONHandleRTCChange(qemuMonitorPtr mon, virJSONValuePtr data) { long long offset = 0; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d4fd4fb..1dc1f3a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -548,6 +548,7 @@ qemuProcessFakeReboot(void *opaque) qemuDomainObjPrivatePtr priv = vm->privateData; virDomainEventPtr event = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + virDomainRunningReason reason = VIR_DOMAIN_RUNNING_BOOTED; int ret = -1; VIR_DEBUG("vm=%p", vm); virObjectLock(vm); @@ -573,8 +574,11 @@ qemuProcessFakeReboot(void *opaque) goto endjob; } + if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_CRASHED) + reason = VIR_DOMAIN_RUNNING_CRASHED; + if (qemuProcessStartCPUs(driver, vm, NULL, - VIR_DOMAIN_RUNNING_BOOTED, + reason, QEMU_ASYNC_JOB_NONE) < 0) { if (virGetLastError() == NULL) virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1269,6 +1273,98 @@ qemuProcessHandlePMSuspendDisk(qemuMonitorPtr mon ATTRIBUTE_UNUSED, return 0; } +static int +qemuProcessHandleGUESTPanicked(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm) +{ + virQEMUDriverPtr driver = qemu_driver; + qemuDomainObjPrivatePtr priv; + virDomainEventPtr event = NULL; + bool isReboot = true; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + + VIR_DEBUG("vm=%p", vm); + + virObjectLock(vm); + + if (!virDomainObjIsActive(vm)) { + VIR_DEBUG("Ignoring GUEST_PANICKED event from inactive domain %s", + vm->def->name); + goto cleanup; + } + + priv = vm->privateData; + + virDomainObjSetState(vm, + VIR_DOMAIN_CRASHED, + VIR_DOMAIN_CRASHED_PANICKED); + + event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_CRASHED, + VIR_DOMAIN_EVENT_CRASHED_PANICKED); + + if (vm->def->onCrash == VIR_DOMAIN_LIFECYCLE_CRASH_PRESERVE) { + VIR_FREE(priv->lockState); + + if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0) + VIR_WARN("Unable to release lease on %s", vm->def->name); + VIR_DEBUG("Preserving lock state '%s'", NULLSTR(priv->lockState)); + + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) { + VIR_WARN("Unable to save status on vm %s after state change", + vm->def->name); + } + + goto cleanup; + } + + if (vm->def->onCrash == VIR_DOMAIN_LIFECYCLE_CRASH_DESTROY) { + isReboot = false; + VIR_INFO("Domain on_crash setting overridden, shutting down"); + } + + qemuDomainSetFakeReboot(driver, vm, isReboot); + + if (isReboot) { + qemuProcessShutdownOrReboot(driver, vm); + } else { + priv->beingDestroyed = true; + + if (qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_FORCE) < 0) { + priv->beingDestroyed = false; + goto cleanup; + } + + priv->beingDestroyed = false; + + if (! virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_CRASHED, 0); + event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_STOPPED, + VIR_DOMAIN_EVENT_STOPPED_CRASHED); + + virDomainAuditStop(vm, "destroyed"); + + if (! vm->persistent) { + qemuDomainRemoveInactive(driver, vm); + vm = NULL; + } + } + +cleanup: + if (vm) + virObjectUnlock(vm); + if (event) + qemuDomainEventQueue(driver, event); + virObjectUnref(cfg); + + return 0; +} static qemuMonitorCallbacks monitorCallbacks = { .destroy = qemuProcessHandleMonitorDestroy, @@ -1289,6 +1385,7 @@ static qemuMonitorCallbacks monitorCallbacks = { .domainPMSuspend = qemuProcessHandlePMSuspend, .domainBalloonChange = qemuProcessHandleBalloonChange, .domainPMSuspendDisk = qemuProcessHandlePMSuspendDisk, + .domainGUESTPanicked = qemuProcessHandleGUESTPanicked, }; static int diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 5ed89d1..4bddefe 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -192,6 +192,8 @@ vshDomainStateReasonToString(int state, int reason) return N_("save canceled"); case VIR_DOMAIN_RUNNING_WAKEUP: return N_("event wakeup"); + case VIR_DOMAIN_RUNNING_CRASHED: + return N_("from crashed"); case VIR_DOMAIN_RUNNING_UNKNOWN: case VIR_DOMAIN_RUNNING_LAST: ; @@ -226,6 +228,8 @@ vshDomainStateReasonToString(int state, int reason) return N_("shutting down"); case VIR_DOMAIN_PAUSED_SNAPSHOT: return N_("creating snapshot"); + case VIR_DOMAIN_PAUSED_GUEST_PANICKED: + return N_("guest panicked"); case VIR_DOMAIN_PAUSED_UNKNOWN: case VIR_DOMAIN_PAUSED_LAST: ; @@ -236,6 +240,8 @@ vshDomainStateReasonToString(int state, int reason) switch ((virDomainShutdownReason) reason) { case VIR_DOMAIN_SHUTDOWN_USER: return N_("user"); + case VIR_DOMAIN_SHUTDOWN_CRASHED: + return N_("crashed"); case VIR_DOMAIN_SHUTDOWN_UNKNOWN: case VIR_DOMAIN_SHUTDOWN_LAST: ; @@ -266,6 +272,8 @@ vshDomainStateReasonToString(int state, int reason) case VIR_DOMAIN_CRASHED: switch ((virDomainCrashedReason) reason) { + case VIR_DOMAIN_CRASHED_PANICKED: + return N_("panicked"); case VIR_DOMAIN_CRASHED_UNKNOWN: case VIR_DOMAIN_CRASHED_LAST: ; -- 1.8.1.4

3 3

[libvirt] [PATCH] Expose all CPU features in host definition
by Don Dugger 28 May '13

28 May '13

I've opened BZ 697141 on this as I would consider it more a bug than a feature request. Anyway, to re-iterate my rationale from the BZ: The virConnectGetCapabilities API describes the host capabilities by returning an XML description that includes the CPU model name and a set of CPU features. The problem is that any features that are part of the CPU model are not explicitly listed, they are assumed to be part of the definition of that CPU model. This makes it extremely difficult for the caller of this API to check for the presence of a specific CPU feature, the caller would have to know what features are part of which CPU models, a very daunting task. This patch solves this problem by having the API return a model name, as it currently does, but it will also explicitly list all of the CPU features that are present. This would make it much easier for a caller of this API to check for specific features. Signed-off-by: Don Dugger <donald.d.dugger(a)intel.com> --- src/cpu/cpu_x86.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 5d479c2..b2e16df 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1296,6 +1296,35 @@ x86GuestData(virCPUDefPtr host, return x86Compute(host, guest, data, message); } +static void +x86AddFeatures(virCPUDefPtr cpu, + struct x86_map *map) +{ + const struct x86_model *candidate; + const struct x86_feature *feature = map->features; + + candidate = map->models; + while (candidate != NULL) { + if (STREQ(cpu->model, candidate->name)) + break; + candidate = candidate->next; + } + if (!candidate) { + VIR_WARN("Odd, %s not a known CPU model\n", cpu->model); + return; + } + while (feature != NULL) { + if (x86DataIsSubset(candidate->data, feature->data)) { + if (virCPUDefAddFeature(cpu, feature->name, VIR_CPU_FEATURE_DISABLE) < 0) { + VIR_WARN("CPU model %s, no room for feature %s", cpu->model, feature->name); + return; + } + } + feature = feature->next; + } + return; +} + static int x86Decode(virCPUDefPtr cpu, @@ -1383,6 +1412,7 @@ x86Decode(virCPUDefPtr cpu, goto out; } + x86AddFeatures(cpuModel, map); cpu->model = cpuModel->model; cpu->vendor = cpuModel->vendor; cpu->nfeatures = cpuModel->nfeatures; -- 1.7.10.4

2 2

[libvirt] Issues with qemu-nbd over AF_UNIX and virDomainAttachDevice
by Deepak C Shetty 27 May '13

27 May '13

Hi List, Facing issues trying to use virDomainAttachDevice API of libvirt from python. Here is what I am trying to do 1) Setup qemu-nbd to export a qcow2 disk image over unix socket on my localhost qemu-nbd -t -k /tmp/mysock2 /home/dpkshetty/work/img/iscsi_disk The above blocks. and i am able to use qemu-system-x86_64 -drive file=nbd:unix:/tmp/mysock2 Just to test it...and it works. Since currently the disk is a blank disk.. I see "No bootable device" and QEMU stops.. whcih is expected So qemu-nbd is setup and working properly 2) >>> print nbdxml <disk type='network' device='disk'> <driver name="qemu" type="qcow2"/> <source protocol="nbd"> <host name="deepakcs-lx" port="10809" transport="unix" socket="/tmp/mysock2" /> </source> <target dev="vdc" bus="virtio" /> </disk> >>> dom.attachDevice(nbdxml) libvir: QEMU error : operation failed: open disk image file failed Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/usr/lib64/python2.7/site-packages/libvirt.py", line 400, in attachDevice if ret == -1: raise libvirtError ('virDomainAttachDevice() failed', dom=self) libvirt.libvirtError: operation failed: open disk image file failed and I see... in the VM's log.... inet_connect_opts: connect(ipv4,deepakcs-lx.local.lan,192.168.1.7,10809): Connection refused 3) I tried usign virsh attach-device.. and the same problem happens 4) firewall/iptables are not running. SELinux is permissive 5) I tried appending -p 1111 to the qemu-nbd cmdline and using port="1111" in the XML.. but that doesn't help, see the same error Questions/Observations : 1) What am i doing wrong here ? In the <host> tag, port attr is mandatory.. but looking at `netstat -nptl` I don't see any process listeing on 10809 or 1111 (when i gave -p 1111 ). So does this mean there is no way for libvirt to just directly open the socket and use it.. Its trying to connect to the port i give in the XML and since no one is listening there.. it fails.. hence the error seen in the VM logs.... Has anybody tryign usign qemu-nbd and attaching that disk as I am doign above ? Is it legal, if not, why ? 2) The VM/Domain is active during all of the above steps. 3) IIUC... for the above usecase... port attr of <host> should be optional... such that libvirt can directly do the equivalent of ` -drive file=nbd:unix:/tmp/mysock2` and add this as a NBD device to the runnign guest/Domain... but currently port and hostname are mandatory. For unix sockets... hostname will always be local...so i feel both hostname & port should be optional for the above usecase ? 4) I haven't yet tried doing the above when guest/Domain is Inactive. 5) I haven't yet tried doing the above usign nbd-server or qemu-nbd usign -c option (which i believe starts qemu-nbd in server mode, but need nbd.ko kernel module to be loaded). I am hoping using either of these might get it working... since we have a process listening on a port on the host... but I wanted to get this working using plain unix sockets.. lightweight and doesn't need nbd.ko Comments/Suggestions appreciated. thanx, deepak

2 9

[libvirt] fix change-media bug on disk block type and support volume type
by Guannan Ren 27 May '13

27 May '13

Resolves:https://bugzilla.redhat.com/show_bug.cgi?id=923053 When cdrom is block type, the virsh change-media failed to insert source info because virsh uses "<source block='/dev/sdb'/>" while the correct name of the attribute for block disks is "dev". Correct XML: <disk type='block' device='cdrom'> <driver name='qemu' type='raw'/> <source dev='/dev/sdb'/> <target dev='vdb' bus='virtio'/> <readonly/> </disk> And, this patch supports cdrom with volume type for change-media command For example: '/var/lib/libvirt/images/boot.iso' is a volume path of 'boot.iso' volume on 'default' pool and the cdrom device has no source. Virsh command: virsh change-media rhel6qcow2 vdb /var/lib/libvirt/images/boot.iso --insert The updated disk XML: <disk type='volume' device='cdrom'> <driver name='qemu' type='raw'/> <source pool='default' volume='boot.iso'/> <target dev='vdb' bus='virtio'/> <readonly/> </disk> Guannan Ren(3) [PATCH 1/3] qemu: throw original error when failing to lookup pool or volume [PATCH 2/3] qemu: support updating pool and volume info when disk is volume type [PATCH 3/3] virsh: fix change-media bug on disk block type and support volume type src/conf/domain_conf.c | 2 +- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/qemu/qemu_conf.c | 11 ++++++++++- src/qemu/qemu_driver.c | 5 +++++ tools/virsh-domain.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++------ 6 files changed, 69 insertions(+), 8 deletions(-)

1 3

Re: [libvirt] [Qemu-devel] [PATCH v3 2/2] net: introduce command to query rx-filter information
by Michael S. Tsirkin 27 May '13

27 May '13

On Fri, May 24, 2013 at 05:20:13PM +0200, Markus Armbruster wrote: > Eric Blake <eblake(a)redhat.com> writes: > > > On 05/24/2013 06:23 AM, Luiz Capitulino wrote: > >>>> I don't think we need this argument. This command is quite simple in its > >>>> response, let's do this filtering in HMP only. > >>> > >>> Event message contains the net client name, management might only want > >>> to query the single net client. > >> > >> The client can do the filtering itself. > > > > If we're arguing that we want this to be as responsive as possible, then > > the less data we send over the wire, the faster management can react to > > the guest's request for a particular NIC. That is, if libvirt is > > listening to events that says NIC2 wants to change rx-filter, libvirt > > would rather do a filtered query where it knows the JSON array of 1 > > element matches NIC2 data, rather than do a global query and search > > through the returned array until it finds NIC2. > > > > Filtering is relatively easy to add, whether you do it in QMP or make > > every client add it. Libvirt will survive if you don't have filtering, > > but I don't see why we can't have it in QMP. Also, if you DO decide to > > rip filtering out of QMP, you STILL need to keep a per-NIC flag. Since > > the events say which NIC is requesting a change, even if the query reads > > all nics, libvirt will only change the macvtap settings of the nic(s) > > for which it has received an event (it doesn't make sense to waste time > > requesting a (no-op) change to macvtap settings on a nic that hasn't > > requested a change). But if you argue that having no filtering in the > > QMP command means that you can get away with a single flag instead of a > > per-nic flag, then you will fail to emit an event for NIC2 if it changes > > in between the time that NIC1 fired an event and libvirt finally does > > the query, and libvirt wouldn't realize that NIC2 also needs a macvtap > > change. > > No disagreement on the need for a per-NIC flag. > > I'm not sure I buy the responsiveness argument. Sure, the fastest I/O > is no I/O, but whether you read and parse 100 bytes or 1000 from a Unix > domain socket once in a great while shouldn't make a difference. > > My main concern is to keep the external interface simple. I'm rather > reluctant to have query commands grow options. > > In a case where we need the "give me everything" query anyway, the "give > me this particular part" option is additional complexity. Needs > justification, say arguments involving throughput, latency or client > complexity. > > Perhaps cases exist where we never want to ask for everything. Then the > "give me everything" query is useless, and the option should be > mandatory. We need the query for macvtap devices. We don't need it for tap devices. In that case you don't want tap device info. Maybe some libvirt guys can tell us whether they prefer a per device query or a global one with info for all NICs? I think for HMP it's best to have nic optional. Is it a good idea to make QMP match HMP closely? -- MST

4 4

[libvirt] [PATCH] qemu: Fix build without gnutls
by Jiri Denemark 27 May '13

27 May '13

"error" label in qemuMigrationCookieGraphicsAlloc is now used unconditionally thanks to VIR_STRDUP. --- Pushed as a build-breaker. src/qemu/qemu_migration.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 73ced73..19b1236 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -324,9 +324,7 @@ qemuMigrationCookieGraphicsAlloc(virQEMUDriverPtr driver, no_memory: virReportOOMError(); -#ifdef WITH_GNUTLS error: -#endif qemuMigrationCookieGraphicsFree(mig); virObjectUnref(cfg); return NULL; -- 1.8.2.1

1 0

[libvirt] two hostdev devices problem
by Dominik Mostowiec 27 May '13

27 May '13

hi, I try to add 2 VF by "hostdev". Networks (vnet0, vnet1) with: <forward mode='hostdev' managed='yes'> <pf dev='eth1'/> ..... Domain: <interface type="network"> <source network="vnet0"/> .... <interface type="network"> <source network="vnet1"/> .... virsh create error: "error: internal error process exited while connecting to monitor: kvm: -device pci-assign,configfd=25,host=01:10.1,id=hostdev0,bus=pci.0,addr=0x4: Duplicate ID 'hostdev0' for device" I am doing somthing wrong? My configs: domain: <domain xmlns:qemu="http://libvirt.org/schemas/domain/qemu/1.0" type="kvm"> <name>one-466</name> <vcpu>1</vcpu> <memory>1781760</memory> <os> <type arch="x86_64">hvm</type> <boot dev="hd"/> </os> <devices> <emulator>/usr/bin/kvm</emulator> <disk type="file" device="disk"> <source file="/nebula/nebula/var//datastores/0/466/disk.0"/> <target dev="hda" bus="virtio"/> <driver name="qemu" type="raw" cache="default"/> </disk> <disk type="file" device="disk"> <source file="/nebula/nebula/var//datastores/0/466/disk.1"/> <target dev="vdb" bus="virtio"/> <driver name="qemu" type="raw" cache="default"/> </disk> <graphics type="vnc" listen="0.0.0.0" port="-1"/> <interface type="network"> <source network="vnet0"/> <mac address="52:54:0a:b1:48:91"/> </interface> <interface type="network"> <source network="vnet1"/> <mac address="52:54:0a:b1:48:92"/> </interface> </devices> <features> <acpi/> </features> <devices><serial type="pty"><source path="/dev/pts/477"/><target port="0"/></serial><console type="pty" tty="/dev/pts/477"><source path="/dev/pts/477"/><target port="0"/></console></devices> </domain> Networks: cat /etc/libvirt/qemu/networks/vnet0.xml  <network> <name>vnet0</name> <uuid>1b6a3506-d9e0-4e1e-bbff-6f700b8dc80b</uuid> <forward mode='hostdev' managed='yes'> <pf dev='eth0'/> </forward> </network> cat /etc/libvirt/qemu/networks/vnet1.xml  <network> <name>vnet1</name> <uuid>11feb088-d609-4177-86d7-ac3b9467a3e8</uuid> <forward mode='hostdev' managed='yes'> <pf dev='eth1'/> </forward> </network> I wrote libvirt-users mailing list too. -- Pozdrawiam Dominik

4 15

[libvirt] [test-API][PATCH] Add 2 emulatorpin cases cover config and live flags
by Wayne Sun 27 May '13

27 May '13

- use pinEmulator to pin domain emulator to host cpu - 2 cases cover config and live flags - cpulist with '^', '-' and ',' is supported to give multiple host cpus Related bug 916493: pinEmulator and emulatorPinInfo should be simple with required params https://bugzilla.redhat.com/show_bug.cgi?id=916493 is fixed, so the test can run successfully now. Signed-off-by: Wayne Sun <gsun(a)redhat.com> --- repos/setVcpus/emulatorpin_config.py | 97 +++++++++++++++++++++++++++++++++++ repos/setVcpus/emulatorpin_live.py | 98 ++++++++++++++++++++++++++++++++++++ 2 files changed, 195 insertions(+) create mode 100644 repos/setVcpus/emulatorpin_config.py create mode 100644 repos/setVcpus/emulatorpin_live.py diff --git a/repos/setVcpus/emulatorpin_config.py b/repos/setVcpus/emulatorpin_config.py new file mode 100644 index 0000000..9b94f98 --- /dev/null +++ b/repos/setVcpus/emulatorpin_config.py @@ -0,0 +1,97 @@ +#!/usr/bin/env python +# Test domain emulator pin with flag VIR_DOMAIN_AFFECT_CONFIG, check +# domain config xml with emulatorpin configuration. + +import re +from xml.dom import minidom + +import libvirt +from libvirt import libvirtError + +from src import sharedmod +from utils import utils + +required_params = ('guestname', 'cpulist',) +optional_params = {} + +def emulatorpin_check(domobj, cpumap): + """check domain config xml with emulatorpin element + """ + guestxml = domobj.XMLDesc(2) + logger.debug("domain %s xml :\n%s" %(domobj.name(), guestxml)) + + doc = minidom.parseString(guestxml) + emulatorpin = doc.getElementsByTagName('emulatorpin') + if not emulatorpin: + logger.error("no emulatorpin element in domain xml") + return 1 + + if not emulatorpin[0].hasAttribute('cpuset'): + logger.error("no cpuset attribute with emulatorpin in domain xml") + return 1 + else: + emulator_attr = emulatorpin[0].getAttributeNode('cpuset') + cpulist = emulator_attr.nodeValue + cpumap_tmp = utils.param_to_tuple(cpulist, maxcpu) + + if cpumap_tmp == cpumap: + logger.info("cpuset is as expected in domain xml") + return 0 + else: + logger.error("cpuset is not as expected in domain xml") + return 1 + +def emulatorpin_config(params): + """pin domain emulator to host cpu with config flag + """ + global logger + logger = params['logger'] + params.pop('logger') + guestname = params['guestname'] + cpulist = params['cpulist'] + + logger.info("the name of virtual machine is %s" % guestname) + logger.info("the given cpulist is %s" % cpulist) + + global maxcpu + maxcpu = utils.get_host_cpus() + logger.info("%s physical cpu on host" % maxcpu) + + conn = sharedmod.libvirtobj['conn'] + + try: + domobj = conn.lookupByName(guestname) + cpumap = utils.param_to_tuple(cpulist, maxcpu) + + if not cpumap: + logger.error("cpulist: Invalid format") + return 1 + + logger.debug("cpumap for emulator pin is:") + logger.debug(cpumap) + + logger.info("pin domain emulator to host cpulist %s with flag: %s" % + (cpulist, libvirt.VIR_DOMAIN_AFFECT_CONFIG)) + domobj.pinEmulator(cpumap, libvirt.VIR_DOMAIN_AFFECT_CONFIG) + + logger.info("check emulator pin info") + ret = domobj.emulatorPinInfo(libvirt.VIR_DOMAIN_AFFECT_CONFIG) + logger.debug("emulator pin info is:") + logger.debug(ret) + if ret == cpumap: + logger.info("emulator pin info is expected") + else: + logger.error("emulator pin info is not expected") + return 1 + except libvirtError, e: + logger.error("libvirt call failed: " + str(e)) + return 1 + + logger.info("check domain emulatorpin configuration in xml") + ret = emulatorpin_check(domobj, cpumap) + if ret: + logger.error("domain emulator pin check failed") + return 1 + else: + logger.info("domain emulator pin check succeed") + return 0 diff --git a/repos/setVcpus/emulatorpin_live.py b/repos/setVcpus/emulatorpin_live.py new file mode 100644 index 0000000..08b7073 --- /dev/null +++ b/repos/setVcpus/emulatorpin_live.py @@ -0,0 +1,98 @@ +#!/usr/bin/env python +# Test domain emulator pin with flag VIR_DOMAIN_AFFECT_LIVE, check +# emulator process status under domain task list on host. + +import re + +import libvirt +from libvirt import libvirtError + +from src import sharedmod +from utils import utils + +required_params = ('guestname', 'cpulist',) +optional_params = {} + +def emulatorpin_check(guestname, cpumap): + """check emulator process status of the running virtual machine + grep Cpus_allowed_list /proc/PID/status + """ + tmp_str = '' + cmd = "cat /var/run/libvirt/qemu/%s.pid" % guestname + status, pid = utils.exec_cmd(cmd, shell=True) + if status: + logger.error("failed to get the pid of domain %s" % guestname) + return 1 + + cmd = "grep Cpus_allowed_list /proc/%s/status" % pid[0] + status, output = utils.exec_cmd(cmd, shell=True) + if status: + logger.error("failed to run command: %s" % cmd) + return 1 + + logger.debug("command '%s' output is:\n%s" % (cmd, output[0])) + + cpulist = output[0].split('\t')[1] + ret = utils.param_to_tuple(cpulist, maxcpu) + + if ret == cpumap: + logger.info("emulator process cpus allowed list is expected") + return 0 + else: + logger.error("emulator process cpus allowed list is not expected") + return 1 + +def emulatorpin_live(params): + """pin domain emulator to host cpu with live flag + """ + global logger + logger = params['logger'] + params.pop('logger') + guestname = params['guestname'] + cpulist = params['cpulist'] + + logger.info("the name of virtual machine is %s" % guestname) + logger.info("the given cpulist is %s" % cpulist) + + global maxcpu + maxcpu = utils.get_host_cpus() + logger.info("%s physical cpu on host" % maxcpu) + + conn = sharedmod.libvirtobj['conn'] + + try: + domobj = conn.lookupByName(guestname) + cpumap = utils.param_to_tuple(cpulist, maxcpu) + if not cpumap: + logger.error("cpulist: Invalid format") + return 1 + + logger.debug("cpumap for emulator pin is:") + logger.debug(cpumap) + + logger.info("pin domain emulator to host cpu %s with flag: %s" % + (cpulist, libvirt.VIR_DOMAIN_AFFECT_LIVE)) + domobj.pinEmulator(cpumap, libvirt.VIR_DOMAIN_AFFECT_LIVE) + + logger.info("check emulator info") + ret = domobj.emulatorPinInfo() + logger.debug("emulator info is:") + logger.debug(ret) + if ret == cpumap: + logger.info("emulator info is expected") + else: + logger.error("emulator info is not expected") + return 1 + + except libvirtError, e: + logger.error("libvirt call failed: " + str(e)) + return 1 + + logger.info("check emulator pin status on host") + ret = emulatorpin_check(guestname, cpumap) + if ret: + logger.error("domain emulator pin failed") + return 1 + else: + logger.info("domain emulator pin succeed") + return 0 -- 1.8.2.1

1 0

[libvirt] [PATCH] qemuOpenVhostNet: Decrease vhostfdSize on open failure
by Michal Privoznik 27 May '13

27 May '13

Currently, if there's an error opening /dev/vhost-net (e.g. because it doesn't exist) but it's not required we proceed with vhostfd array filled with -1 and vhostfdSize unchanged. Later, when constructing the qemu command line only non-negative items within vhostfd array are taken into account. This means, vhostfdSize may be greater than the actual count of non-negative items in vhostfd array. This results in improper command line arguments being generated, e.g.: -netdev tap,fd=21,id=hostnet0,vhost=on,vhostfd=(null) --- src/qemu/qemu_command.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 434f5a7..d969f88 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -486,6 +486,8 @@ qemuOpenVhostNet(virDomainDefPtr def, "but is unavailable")); goto error; } + i--; + (*vhostfdSize)--; } } virDomainAuditNetDevice(def, net, "/dev/vhost-net", *vhostfdSize); @@ -6560,12 +6562,10 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, } for (i = 0; i < vhostfdSize; i++) { - if (vhostfd[i] >= 0) { - virCommandTransferFD(cmd, vhostfd[i]); - if (virAsprintf(&vhostfdName[i], "%d", vhostfd[i]) < 0) { - virReportOOMError(); - goto cleanup; - } + virCommandTransferFD(cmd, vhostfd[i]); + if (virAsprintf(&vhostfdName[i], "%d", vhostfd[i]) < 0) { + virReportOOMError(); + goto cleanup; } } -- 1.8.2.1

2 2

[libvirt] [PATCH 0/2] Fix the display of *virsh vcpuinfo* command.
by yangdongsheng 27 May '13

27 May '13

Hi, I found some confusion in display of commad *virsh vcpuinfo domname* . There is no diffrence between a cpu offline and a cpu online but not be set for the affinity of vcpu, both are '-'. But users will find the difference of them in using them for a same action (such as vcpupin) and be confused. Example: [root@yds-pc ~]# echo "0" >/sys/devices/system/cpu/cpu2/online [root@yds-pc ~]# cat /proc/cpuinfo |grep processor processor : 0 processor : 1 processor : 3 [root@yds-pc ~]# virsh vcpupin virt-tests-vm1 0 0,3 [root@yds-pc ~]# virsh vcpuinfo virt-tests-vm1 VCPU: 0 CPU: 0 State: running CPU time: 14.8s CPU Affinity: y--y *There is no diff between cpu1 and cpu2 (both '-')* BUT: [root@yds-pc ~]# virsh vcpupin virt-tests-vm1 0 1 [root@yds-pc ~]# echo $? 0 [root@yds-pc ~]# virsh vcpupin virt-tests-vm1 0 2 error: Requested operation is not valid: failed to set cpuset.cpus in cgroup for vcpu 0 [root@yds-pc ~]# echo $? 1 So, I think we should show the cpu in different state with different symbol. In this patchset, I introduce a symbol 'x' (maybe there is a better one :)) to display the cpu is offline on host. Just like: [root@yds-pc ~]# virsh vcpuinfo virt-tests-vm1 VCPU: 0 CPU: 0 State: running CPU time: 14.8s CPU Affinity: y-xy yangdongsheng (2): tool/virsh-domain.c: Add a vshNodeGetCPUMap function to get cpumap of host. tool/virsh-domain.c: Fix the display of Affinity in function cmdVcpuinfo. tools/virsh-domain.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 75 insertions(+), 3 deletions(-) -- 1.7.10.1

4 6

[libvirt] virCommandWait internal error Child process unexpected exit status 1
by jj 25 May '13

25 May '13

I am use libvirt master branch to test LXC , the lastest commit of the master branch is 03d813bbcd7b4a18360105500672b84d985dd889, by Marek Marczykowski <marmarek(a)invisiblethingslab.com> on Thu May 23 02:01:30 2013 +0200, root@wheezy:/tmp# cat test.xml <domain type='lxc'> <name>vmtest</name> <memory>500000</memory> <os> <type>exe</type> <cmdline>console=ttyS0</cmdline> <init>/sbin/init</init> </os> <vcpu>1</vcpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> <filesystem type="mount"> <source dir="/tmp/x"/> <target dir="/"/> </filesystem> <emulator>/usr/libexec/libvirt_lxc</emulator> <interface type='network'> <source network='default'/> </interface> <console type='pty' /> </devices> </domain> root@wheezy:/tmp# virsh -c lxc:/// define test.xml Domain vmtest defined from test.xml root@wheezy:/tmp# virsh -c lxc:/// start vmtest error: Failed to start domain vmtest error: internal error Child process (PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin LIBVIRT_DEBUG=3 LIBVIRT_LOG_OUTPUTS=3:stderr /usr/libexec/libvirt_lxc --name vmtest --console 19 --security=none --handshake 22 --background --veth veth7) unexpected exit status 1 below's a libvirtd log 2013-05-25 14:46:58.727+0000: 7865: error : virCommandWait:2354 : internal error Child process (PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin LIBVIRT_DEBUG=3 LIBVIRT_LOG_OUTPUTS=3:stderr /usr/libexec/libvirt_lxc --name vmtest --console 19 --security=none --handshake 22 --background --veth veth7) unexpected exit status 1

1 0

Re: [libvirt] [Qemu-devel] [PATCH] qapi-schema.json: Reformat TargetType enum to one-per-line
by Anthony Liguori 25 May '13

25 May '13

Peter Maydell <peter.maydell(a)linaro.org> writes: > On 22 May 2013 14:15, Anthony Liguori <aliguori(a)us.ibm.com> wrote: >> Paolo Bonzini <pbonzini(a)redhat.com> writes: >>> You >>> don't need to know what targets were supported in the version that you >>> compiled from. Only one target is supported in this executable >>> anyway. >> >> It seems useful to me. One day we may support multiple targets per >> executable. > > Why would you care about which architectures the executable supports? > What you actually want to know is which machine models are supported; > whether board foo happens to be ARM or PPC isn't really very interesting > IMHO. That's a very good point. It was the libvirt folks that requested this. Perhaps they can shed some light on the logic? Regards, Anthony Liguori > > -- PMM

5 4

[libvirt] [PATCH] qemu: save domain state to XML after reboot
by root 25 May '13

25 May '13

Currently qemuDomainReboot() does reboot in two phases: qemuMonitorSystemPowerdown() and qemuProcessFakeReboot(). qemuMonitorSystemPowerdown() shutdowns the domain and saves domain state/reason as VIR_DOMAIN_SHUTDOWN_UNKNOWN. qemuProcessFakeReboot() sets domain state/reason to VIR_DOMAIN_RESUMED_UNPAUSED but does not save domain state changes. Subsequent restart of libvirtd leads to restoring domain state/reason to saved that is VIR_DOMAIN_SHUTDOWN_UNKNOWN and to automatic shutdown of the domain. This commit adds virDomainSaveStatus() into qemuProcessFakeReboot() to avoid unexpected shutdowns. --- src/qemu/qemu_process.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e5b4679..cbe35ac 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -547,6 +547,7 @@ qemuProcessFakeReboot(void *opaque) virDomainObjPtr vm = opaque; qemuDomainObjPrivatePtr priv = vm->privateData; virDomainEventPtr event = NULL; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); int ret = -1; VIR_DEBUG("vm=%p", vm); virObjectLock(vm); @@ -585,6 +586,11 @@ qemuProcessFakeReboot(void *opaque) VIR_DOMAIN_EVENT_RESUMED, VIR_DOMAIN_EVENT_RESUMED_UNPAUSED); + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) { + VIR_WARN("Unable to save status on vm %s after state change", + vm->def->name); + } + ret = 0; endjob: @@ -601,6 +607,7 @@ cleanup: } if (event) qemuDomainEventQueue(driver, event); + virObjectUnref(cfg); } -- 1.8.2.1

3 2

[libvirt] virStream double unref in virChrdevOpen()
by Sergey Fionov 25 May '13

25 May '13

Hello, There is double unref virChrdevOpen() (src/conf/virchrdev.c) when error occured. if (virStreamRef(st) < 0) { virMutexUnlock(&devs->lock); return -1; } ... if (virHashAddEntry(devs->hash, path, st) < 0) goto error; ... if (virFDStreamOpenFile(st, path, 0, 0, O_RDWR) < 0) /* error occured here */ goto error; ... error: virStreamFree(st); virHashRemoveEntry(devs->hash, path); stream is virStreamRef'ed 1 time but if it is successfully placed into hash then it will be unreferenced 2 times - in virStreamFree() and virHashRemoveEntry()'s dataFree callback. That leads to dispose stream object and segmentation fault due to use after free. Steps to reproduce: # hide the /dev/pts to throw an error in virFDStreamOpenFile() $ mount -t tmpfs empty-devpts /dev/pts $ virsh console a111 Connected to domain a111 Escape character is ^] error: End of file while reading data: Input/output error error: One or more references were leaked after disconnect from the hypervisor error: Failed to reconnect to the hypervisor (libvirtd segfaults) $ umount empty-devpts wbr, Sergey.

2 1

[libvirt] [PATCH] esx: Fix dynamic VI object type detection
by Matthias Bolte 25 May '13

25 May '13

VI objects support inheritance with subtype polymorphism. For example the FileInfo object type is extended by FloppyImageFileInfo, FolderFileInfo etc. Then SearchDatastore_Task returns an array of FileInfo objects and depending on the represented file the FileInfo is actually a FolderFileInfo or FloppyImageFileInfo etc. The actual type information is stored as XML attribute that allows clients such as libvirt to distinguish between the actual types. esxVI_GetActualObjectType is used to extract the actual type. I assumed that this mechanism would be used for all VI object types that have subtypes. But this is not the case. It seems only to be used for types that are actually used as generic base type such as FileInfo. But it is not used for types that got extended later such as ElementDescription that was extended by ExtendedElementDescription (added in vSphere API 4.0) or that are not meant to be used with subtype polymorphism. This breaks the deserialization of types that contain ElementDescription properties such as PerfCounterInfo or ChoiceOption, because the code expects an ElementDescription object to have an XML attribute named type that is not present, since ExtendedElementDescription was added to the esx_vi_generator.input in commit 60f0f55ee4686fecbffc5fb32f90863427d02a14. This in turn break virtual machine question handling and auto answering. Fix this by using the base type if no XML type attribute is present. --- src/esx/esx_vi_types.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c index 1a26556..c55396c 100644 --- a/src/esx/esx_vi_types.c +++ b/src/esx/esx_vi_types.c @@ -720,10 +720,9 @@ esxVI_GetActualObjectType(xmlNodePtr node, esxVI_Type baseType, BAD_CAST "http://www.w3.org/2001/XMLSchema-instance"); if (type == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("%s is missing 'type' property"), - esxVI_Type_ToString(baseType)); - return -1; + // no actual type specified, use base type instead + *actualType = baseType; + return 0; } *actualType = esxVI_Type_FromString(type); -- 1.7.9.5

2 2

[libvirt] [PATCH] spec: Build vbox packages only for x86 architectures
by Viktor Mihajlovski 25 May '13

25 May '13

Commit 6ab6bc19f03513fd87d29ecfd405bb7f4a7de114 has introduced separate daemon/driver packages for vbox. These should only be built for x86 architectures which is done hereby. Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com> --- libvirt.spec.in | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libvirt.spec.in b/libvirt.spec.in index e1c5f49..7abe9fe 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -130,6 +130,11 @@ %define with_libxl 0 %endif +# vbox is available only on i386 x86_64 +%ifnarch %{ix86} x86_64 + %define with_vbox 0 +%endif + # Numactl is not available on s390[x] and ARM %ifarch s390 s390x %{arm} %define with_numactl 0 -- 1.7.9.5

2 1

[libvirt] [PATCH] syntax-check: ignore all quoted strings in bracket-spacing
by Ján Tomko 24 May '13

24 May '13

Ignore strings with an escaped double quote too. This removes the need for special handling of quotes in the semicolon check. --- build-aux/bracket-spacing.pl | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/build-aux/bracket-spacing.pl b/build-aux/bracket-spacing.pl index fbe6666..a99edeb 100755 --- a/build-aux/bracket-spacing.pl +++ b/build-aux/bracket-spacing.pl @@ -31,8 +31,8 @@ foreach my $file (@ARGV) { while (defined (my $line = <FILE>)) { my $data = $line; - # Kill any quoted strongs - $data =~ s,".*?","XXX",g; + # Kill any quoted strings + $data =~ s,"([^\\\"]|\\.)+","XXX",g; # Kill any C++ style comments $data =~ s,//.*$,//,; @@ -120,14 +120,9 @@ foreach my $file (@ARGV) { # errno == EINTR) # ; # - # 3) ";" is inside double-quote, I.e, as part of const string. E.g. - # printf("%s", "a ; b\n"); while ($data =~ /[^;\s]\s+;/) { - # Inside the double-quote - if ($data !~ /"[^"]*\s;/) { - print "$file:$.: $line"; - $ret = 1; - } + print "$file:$.: $line"; + $ret = 1; last; } } -- 1.8.1.5

2 3

[libvirt] [PATCH 1/2] virStrndup: Accept negative values as string length
by Michal Privoznik 24 May '13

24 May '13

It may shorten the code a bit as the following pattern: VIR_STRNDUP(dst, src, cond ? n : strlen(src)) is used on several places among our code. However, we can move the strlen into virStrndup and thus write just: VIR_STRNDUP(dst, src, cond ? n : -1) --- src/util/virstring.c | 7 ++++++- src/util/virstring.h | 11 ++++++++--- tests/virstringtest.c | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/src/util/virstring.c b/src/util/virstring.c index fcbb375..b244e6c 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -568,12 +568,15 @@ virStrdup(char **dest, * caller's body where virStrndup is called from. Consider * using VIR_STRNDUP which sets these automatically. * + * In case @n is smaller than zero, the whole @src string is + * copied. + * * Returns: 0 for NULL src, 1 on successful copy, -1 otherwise. */ int virStrndup(char **dest, const char *src, - size_t n, + ssize_t n, bool report, int domcode, const char *filename, @@ -582,6 +585,8 @@ virStrndup(char **dest, { if (!src) return 0; + if (n < 0) + n = strlen(src); if (!(*dest = strndup(src, n))) { if (report) virReportOOMErrorFull(domcode, filename, funcname, linenr); diff --git a/src/util/virstring.h b/src/util/virstring.h index 534ce91..7063fe4 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -93,7 +93,7 @@ int virStrdup(char **dest, const char *src, bool report, int domcode, const char *filename, const char *funcname, size_t linenr) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1); -int virStrndup(char **dest, const char *src, size_t n, bool report, int domcode, +int virStrndup(char **dest, const char *src, ssize_t n, bool report, int domcode, const char *filename, const char *funcname, size_t linenr) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1); @@ -132,7 +132,9 @@ int virStrndup(char **dest, const char *src, size_t n, bool report, int domcode, * @n: the maximum number of bytes to copy * * Duplicate @src string and store it into @dst. If @src is longer than @n, - * only @n bytes are copied and terminating null byte '\0' is added. + * only @n bytes are copied and terminating null byte '\0' is added. If @n + * is a negative number, then the whole @src string is copied. That is + * VIR_STRDUP(dst, src) and VIR_STRNDUP(dst, src, -1) are equal. * * This macro is safe to use on arguments with side effects. * @@ -150,7 +152,10 @@ int virStrndup(char **dest, const char *src, size_t n, bool report, int domcode, * @n: the maximum number of bytes to copy * * Duplicate @src string and store it into @dst. If @src is longer than @n, - * only @n bytes are copied and terminating null byte '\0' is added. + * only @n bytes are copied and terminating null byte '\0' is added. If @n + * is a negative number, then the whole @src string is copied. That is + * VIR_STRDUP_QUIET(dst, src) and VIR_STRNDUP_QUIET(dst, src, -1) are + * equal. * * This macro is safe to use on arguments with side effects. * diff --git a/tests/virstringtest.c b/tests/virstringtest.c index da06c0f..3d0b55b 100644 --- a/tests/virstringtest.c +++ b/tests/virstringtest.c @@ -196,6 +196,39 @@ cleanup: return ret; } +static int +testStrndupNegative(const void *opaque ATTRIBUTE_UNUSED) +{ + int ret = -1; + char *dst; + const char *src = "Hello world"; + int value; + + if ((value = VIR_STRNDUP(dst, src, 5)) != 1) { + fprintf(stderr, "unexpected virStrndup result %d, expected 1\n", value); + goto cleanup; + } + + if (STRNEQ_NULLABLE(dst, "Hello")) { + fprintf(stderr, "unexpected content '%s'", dst); + goto cleanup; + } + + if ((value = VIR_STRNDUP(dst, src, -1)) != 1) { + fprintf(stderr, "unexpected virStrndup result %d, expected 1\n", value); + goto cleanup; + } + + if (STRNEQ_NULLABLE(dst, src)) { + fprintf(stderr, "unexpected content '%s'", dst); + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FREE(dst); + return ret; +} static int mymain(void) @@ -245,6 +278,9 @@ mymain(void) if (virtTestRun("strdup", 1, testStrdup, NULL) < 0) ret = -1; + if (virtTestRun("strdup", 1, testStrndupNegative, NULL) < 0) + ret = -1; + return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.8.2.1

2 5

[libvirt] [PATCH v2] Fix blkdeviotune for shutoff domain
by Martin Kletzander 24 May '13

24 May '13

Function qemuDomainSetBlockIoTune() was checking QEMU capabilities even when !(flags & VIR_DOMAIN_AFFECT_LIVE) and the domain was shutoff, resulting in the following problem: virsh # domstate asdf; blkdeviotune asdf vda --write-bytes-sec 100 shut off error: Unable to change block I/O throttle error: unsupported configuration: block I/O throttling not supported with this QEMU binary Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=965016 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- Notes: Moved the job starting before anything happens src/qemu/qemu_driver.c | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3df26b8..4a76f14 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13918,26 +13918,17 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, if (!(vm = qemuDomObjFromDomain(dom))) return -1; + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + priv = vm->privateData; cfg = virQEMUDriverGetConfig(driver); if (!(caps = virQEMUDriverGetCapabilities(driver, false))) - goto cleanup; - - if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("block I/O throttling not supported with this " - "QEMU binary")); - goto cleanup; - } - - device = qemuDiskPathToAlias(vm, disk, &idx); - if (!device) { - goto cleanup; - } + goto endjob; - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) - goto cleanup; + if (!(device = qemuDiskPathToAlias(vm, disk, &idx))) + goto endjob; if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags, &persistentDef) < 0) @@ -13987,6 +13978,13 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, } if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("block I/O throttling not supported with this " + "QEMU binary")); + goto endjob; + } + /* If the user didn't specify bytes limits, inherit previous * values; likewise if the user didn't specify iops * limits. */ @@ -14011,9 +14009,6 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_CONFIG) { sa_assert(persistentDef); - idx = virDomainDiskIndexByName(persistentDef, disk, true); - if (idx < 0) - goto endjob; oldinfo = &persistentDef->disks[idx]->blkdeviotune; if (!set_bytes) { info.total_bytes_sec = oldinfo->total_bytes_sec; @@ -14035,7 +14030,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, } endjob: - if (qemuDomainObjEndJob(driver, vm) == 0) + if (!qemuDomainObjEndJob(driver, vm)) vm = NULL; cleanup: -- 1.8.2.1

2 2

[libvirt] [PATCH] nwfilter: Remove error report in virNWFilterDHCPSnoopEnd
by Stefan Berger 24 May '13

24 May '13

Remove error reporting when calling the virNWFilterDHCPSnoopEnd function with an interface for which no thread is snooping traffic. Document the usage of this function. Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com> --- src/nwfilter/nwfilter_dhcpsnoop.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) Index: libvirt/src/nwfilter/nwfilter_dhcpsnoop.c =================================================================== --- libvirt.orig/src/nwfilter/nwfilter_dhcpsnoop.c +++ libvirt/src/nwfilter/nwfilter_dhcpsnoop.c @@ -2117,6 +2117,16 @@ err_exit: return -1; } +/** + * End a DHCP snoop thread on the given interface or end all + * DHCP snoop threads. + * + * @ifname: Name of an interface or NULL to stop all snoop threads + * + * It is not an error to call this function with an interface name + * for which no thread is snooping traffic. In this case the call will + * be a no-op. + */ void virNWFilterDHCPSnoopEnd(const char *ifname) { @@ -2130,11 +2140,8 @@ virNWFilterDHCPSnoopEnd(const char *ifna if (ifname) { ifkey = (char *)virHashLookup(virNWFilterSnoopState.ifnameToKey, ifname); - if (!ifkey) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("ifname \"%s\" not in key map"), ifname); + if (!ifkey) goto cleanup; - } ignore_value(virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey, ifname));

2 2

[libvirt] [PATCH] cgroups: Do not enforce nonexistent controllers
by Viktor Mihajlovski 24 May '13

24 May '13

Currently, the controllers argument to virCgroupDetect acts both as a result filter and a required controller specification, which is a bit overloaded. If both functionalities are needed, it would be better to have them seperated into a filter and a requirement mask. The only situation where it is used today is to ensure that only CPU related controllers are used for the VCPU directories. But here we clearly do not want to enforce the existence of cpu, cpuacct and specifically not cpuset at the same time. This commit changes the semantics of controllers to "filter only". Should a required mask ever be needed, more work will have to be done. Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com> --- src/util/vircgroup.c | 8 ++++---- tests/vircgrouptest.c | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index cc144a5..4fe0944 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -375,18 +375,18 @@ static int virCgroupDetect(virCgroupPtr group, } if (controllers >= 0) { - VIR_DEBUG("Validating controllers %d", controllers); + VIR_DEBUG("Filtering controllers %d", controllers); for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { VIR_DEBUG("Controller '%s' wanted=%s, mount='%s'", virCgroupControllerTypeToString(i), (1 << i) & controllers ? "yes" : "no", NULLSTR(group->controllers[i].mountPoint)); if (((1 << i) & controllers)) { - /* Ensure requested controller is present */ + /* Remove non-existent controllers */ if (!group->controllers[i].mountPoint) { - VIR_DEBUG("Requested controlled '%s' not mounted", + VIR_DEBUG("Requested controller '%s' not mounted, ignoring", virCgroupControllerTypeToString(i)); - return -ENOENT; + controllers &= ~(1 << i); } } else { /* Check whether a request to disable a controller diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index b00a187..262eb8b 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -176,7 +176,7 @@ static int testCgroupNewForDriver(const void *args ATTRIBUTE_UNUSED) /* Asking for impossible combination since devices is not mounted */ if ((rv = virCgroupNewDriver("lxc", true, (1 << VIR_CGROUP_CONTROLLER_DEVICES), - &cgroup)) != -ENOENT) { + &cgroup)) != -ENXIO) { fprintf(stderr, "Should not have created LXC cgroup: %d\n", -rv); goto cleanup; } @@ -280,7 +280,7 @@ static int testCgroupNewForPartition(const void *args ATTRIBUTE_UNUSED) /* Asking for impossible combination since devices is not mounted */ if ((rv = virCgroupNewPartition("/virtualmachines", true, (1 << VIR_CGROUP_CONTROLLER_DEVICES), - &cgroup)) != -ENOENT) { + &cgroup)) != -ENXIO) { fprintf(stderr, "Should not have created /virtualmachines cgroup: %d\n", -rv); goto cleanup; } -- 1.7.9.5

2 2

[libvirt] [PATCH] Fix blkdeviotune for shutoff domain
by Martin Kletzander 24 May '13

24 May '13

Function qemuDomainSetBlockIoTune() was creating job and checking QEMU capabilities even when !(flags & VIR_DOMAIN_AFFECT_LIVE) and the domain was shutoff. Fix this with a little cleanup. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=965016 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/qemu/qemu_driver.c | 52 ++++++++++++++++++++++---------------------------- 1 file changed, 23 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3df26b8..49432f0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13921,27 +13921,12 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, priv = vm->privateData; cfg = virQEMUDriverGetConfig(driver); - if (!(caps = virQEMUDriverGetCapabilities(driver, false))) - goto cleanup; - - if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("block I/O throttling not supported with this " - "QEMU binary")); - goto cleanup; - } - - device = qemuDiskPathToAlias(vm, disk, &idx); - if (!device) { - goto cleanup; - } - - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + if (!(device = qemuDiskPathToAlias(vm, disk, &idx))) goto cleanup; if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags, &persistentDef) < 0) - goto endjob; + goto cleanup; for (i = 0; i < nparams; i++) { virTypedParameterPtr param = &params[i]; @@ -13976,17 +13961,30 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, (info.total_bytes_sec && info.write_bytes_sec)) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("total and read/write of bytes_sec cannot be set at the same time")); - goto endjob; + goto cleanup; } if ((info.total_iops_sec && info.read_iops_sec) || (info.total_iops_sec && info.write_iops_sec)) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("total and read/write of iops_sec cannot be set at the same time")); - goto endjob; + goto cleanup; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) + goto cleanup; + + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("block I/O throttling not supported with this " + "QEMU binary")); + goto cleanup; + } + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + /* If the user didn't specify bytes limits, inherit previous * values; likewise if the user didn't specify iops * limits. */ @@ -14004,16 +14002,16 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, &info); qemuDomainObjExitMonitor(driver, vm); - if (ret < 0) - goto endjob; + if (ret < 0) { + if (!qemuDomainObjEndJob(driver, vm)) + vm = NULL; + goto cleanup; + } vm->def->disks[idx]->blkdeviotune = info; } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { sa_assert(persistentDef); - idx = virDomainDiskIndexByName(persistentDef, disk, true); - if (idx < 0) - goto endjob; oldinfo = &persistentDef->disks[idx]->blkdeviotune; if (!set_bytes) { info.total_bytes_sec = oldinfo->total_bytes_sec; @@ -14030,14 +14028,10 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, if (ret < 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Write to config file failed")); - goto endjob; + goto cleanup; } } -endjob: - if (qemuDomainObjEndJob(driver, vm) == 0) - vm = NULL; - cleanup: VIR_FREE(device); if (vm) -- 1.8.2.1

2 1

[libvirt] [PATCH] virsh: Fix regression of vol-resize
by Osier Yang 24 May '13

24 May '13

Introduced by commit 1daa4ba33acf. vshCommandOptStringReq returns 0 on *success* or the option is not required && not present, both are right result. Error out when returning 0 is not correct. --- tools/virsh-volume.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index a222cca..e16a385 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -1076,7 +1076,7 @@ cmdVolResize(vshControl *ctl, const vshCmd *cmd) if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", NULL))) return false; - if (vshCommandOptStringReq(ctl, cmd, "capacity", &capacityStr) <= 0) + if (vshCommandOptStringReq(ctl, cmd, "capacity", &capacityStr) < 0) goto cleanup; virSkipSpaces(&capacityStr); if (*capacityStr == '-') { -- 1.8.1.4

2 2

[libvirt] [PATCH v4 00/13] Introduce VIR_STRDUP
by Michal Privoznik 24 May '13

24 May '13

Previously this aimed on dropping OOM report from almost everywhere (v1), then just prepare the ground (v2). Now, it just aims on introducing VIR_STRDUP and VIR_STRNDUP which do report OOM error. This will cause rebase conflicts, for sure. Sorry. Patches available at: git://gitorious.org/~zippy2/libvirt/michal-staging.git branch oom5 Michal Privoznik (13): Adapt to VIR_STRDUP and VIR_STRNDUP in src/conf/* Adapt to VIR_STRDUP and VIR_STRNDUP in src/openvz/* Change virConnectDomainEventGraphicsCallback signature _virConnectCredential: turn @prompt into char * Adapt to VIR_STRDUP and VIR_STRNDUP in src/qemu/* Adapt to VIR_STRDUP and VIR_STRNDUP in src/remote/* Adapt to VIR_STRDUP and VIR_STRNDUP in src/rpc/* Adapt to VIR_STRDUP and VIR_STRNDUP in src/security/* Adapt to VIR_STRDUP and VIR_STRNDUP in src/util/* Adapt to VIR_STRDUP and VIR_STRNDUP in src/xen/* Adapt to VIR_STRDUP and VIR_STRNDUP in src/xenapi/* Adapt to VIR_STRDUP and VIR_STRNDUP in src/xenxs/* Introduce syntax-check rule to prefer VIR_STRDUP over strdup cfg.mk | 8 + include/libvirt/libvirt.h.in | 16 +- src/conf/capabilities.c | 30 +-- src/conf/cpu_conf.c | 20 +- src/conf/domain_conf.c | 119 +++-------- src/conf/domain_event.c | 41 ++-- src/conf/node_device_conf.c | 28 ++- src/conf/nwfilter_conf.c | 17 +- src/conf/nwfilter_params.c | 34 +--- src/conf/snapshot_conf.c | 11 +- src/conf/storage_conf.c | 13 +- src/conf/virchrdev.c | 12 +- src/openvz/openvz_conf.c | 45 ++-- src/openvz/openvz_driver.c | 29 +-- src/qemu/qemu_capabilities.c | 79 +++----- src/qemu/qemu_cgroup.c | 4 +- src/qemu/qemu_command.c | 428 +++++++++++++++------------------------ src/qemu/qemu_conf.c | 64 +++--- src/qemu/qemu_domain.c | 26 +-- src/qemu/qemu_driver.c | 129 ++++-------- src/qemu/qemu_hotplug.c | 15 +- src/qemu/qemu_migration.c | 17 +- src/qemu/qemu_monitor_json.c | 63 ++---- src/qemu/qemu_monitor_text.c | 15 +- src/qemu/qemu_process.c | 64 +++--- src/remote/remote_driver.c | 116 ++++------- src/rpc/gendispatch.pl | 21 +- src/rpc/virnetclient.c | 16 +- src/rpc/virnetmessage.c | 27 ++- src/rpc/virnetsaslcontext.c | 6 +- src/rpc/virnetserver.c | 6 +- src/rpc/virnetserverclient.c | 10 +- src/rpc/virnetservermdns.c | 6 +- src/rpc/virnetsocket.c | 10 +- src/rpc/virnetsshsession.c | 78 ++++--- src/rpc/virnettlscontext.c | 26 +-- src/security/security_apparmor.c | 20 +- src/security/security_dac.c | 21 +- src/security/security_nop.c | 7 +- src/security/security_selinux.c | 79 ++------ src/security/virt-aa-helper.c | 4 +- src/util/virauth.c | 17 +- src/util/virauthconfig.c | 8 +- src/util/virbitmap.c | 9 +- src/util/vircgroup.c | 37 ++-- src/util/vircommand.c | 33 ++- src/util/virconf.c | 34 +--- src/util/virdnsmasq.c | 22 +- src/util/virebtables.c | 34 ++-- src/util/virebtables.h | 2 +- src/util/virerror.c | 19 +- src/util/virfile.c | 28 ++- src/util/virhash.c | 5 +- src/util/viridentity.c | 14 +- src/util/virinitctl.c | 4 +- src/util/viriptables.c | 4 +- src/util/virjson.c | 18 +- src/util/virkeyfile.c | 13 +- src/util/virlockspace.c | 25 +-- src/util/virlog.c | 27 +-- src/util/virnetdevmacvlan.c | 16 +- src/util/virnetdevtap.c | 11 +- src/util/virnetdevvportprofile.c | 4 +- src/util/virobject.c | 16 +- src/util/virpci.c | 13 +- src/util/virscsi.c | 6 +- src/util/virsexpr.c | 37 +--- src/util/virsocketaddr.c | 9 +- src/util/virstoragefile.c | 18 +- src/util/virstring.c | 17 +- src/util/virsysinfo.c | 293 +++++++++++---------------- src/util/virtypedparam.c | 14 +- src/util/viruri.c | 58 +++--- src/util/virutil.c | 57 ++---- src/util/virxml.c | 5 +- src/xen/block_stats.c | 16 +- src/xen/xen_driver.c | 9 +- src/xen/xen_hypervisor.c | 19 +- src/xen/xen_inotify.c | 15 +- src/xen/xend_internal.c | 68 ++----- src/xen/xm_internal.c | 9 +- src/xen/xs_internal.c | 44 ++-- src/xenapi/xenapi_driver.c | 59 +++--- src/xenapi/xenapi_utils.c | 39 ++-- src/xenxs/xen_sxpr.c | 200 ++++++++---------- src/xenxs/xen_xm.c | 91 ++++----- 86 files changed, 1244 insertions(+), 2032 deletions(-) -- 1.8.2.1

4 44

[libvirt] [PATCH] virCgroupAddTaskStrController: s/-1/-ENOMEM/
by Michal Privoznik 24 May '13

24 May '13

Within whole vircgroup.c we 'return -errno', e.g. 'return -ENOMEM'. However, in this specific function virCgroupAddTaskStrController we weren't returning -ENOMEM but -1 despite fact that later in the function we are returning one of errno values indeed. --- Notes: Pushing under trivial rule. This change has been sort of ACKed previously though: https://www.redhat.com/archives/libvir-list/2013-May/msg01731.html However, Eric suggested to be pushed as a separate patch. src/util/vircgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index cc144a5..13b7332 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1028,7 +1028,7 @@ static int virCgroupAddTaskStrController(virCgroupPtr group, char *endp; if (!(str = strdup(pidstr))) - return -1; + return -ENOMEM; cur = str; while (*cur != '\0') { -- 1.8.2.1

1 0

[libvirt] [libvirt-sandbox][PATCH] Add missing get_unit_path function into class Container
by Alex Jia 24 May '13

24 May '13

RHBZ:https://bugzilla.redhat.com/show_bug.cgi?id=966307 Signed-off-by: Alex Jia <ajia(a)redhat.com> --- With the patch, we can successfully clone an existing Secure container, but I met a Warning, I think it may be a LibvirtGObject question, # virt-sandbox-service clone http1 clonebox Created sandbox container dir /var/lib/libvirt/filesystems/clonebox Created unit file /etc/libvirt-sandbox/services/clonebox.sandbox /usr/lib64/python2.7/site-packages/gi/types.py:47: Warning: g_key_file_set_string: assertion `string != NULL' failed return info.invoke(*args, **kwargs) Created sandbox config /etc/libvirt-sandbox/services/clonebox.sandbox bin/virt-sandbox-service | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service index 4496b29..2595ea2 100755 --- a/bin/virt-sandbox-service +++ b/bin/virt-sandbox-service @@ -60,6 +60,7 @@ class Container: DEFAULT_PATH = "/var/lib/libvirt/filesystems" DEFAULT_IMAGE = "/var/lib/libvirt/images/%s.raw" SELINUX_FILE_TYPE = "svirt_lxc_file_t" + DEFAULT_UNIT = "/etc/systemd/system/%s_sandbox.service" def __init__(self, name=None, uri = "lxc:///", path = DEFAULT_PATH, config=None, create=False): self.uri = uri @@ -132,6 +133,11 @@ class Container: name = self.get_name() return self.DEFAULT_IMAGE % name + def get_unit_path(self, name = None): + if not name: + name = self.get_name() + return self.DEFAULT_UNIT % name + def set_image(self, size): self.use_image = True self.size = size * MB -- 1.7.1

1 0

[libvirt] [PATCH] xen: Resolve Coverity FORWARD_NULL issue
by John Ferlan 24 May '13

24 May '13

Commit '18b14012' refactored the Xen code resulting in a Coverity warning about possible NULL reference if the path where the XM driver takes puts the def on it's list. Moved/duplicated the virGetDomain() call to pacify the possible NULL deref. NOTE: I did ping Dan B, but he is away, so rather than wait or take the chance this ends up in the upcoming release, I made the change. --- src/xen/xen_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index cc6e486..9eab3fc 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1658,13 +1658,14 @@ xenUnifiedDomainDefineXML(virConnectPtr conn, const char *xml) if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { if (xenXMDomainDefineXML(conn, def) < 0) goto cleanup; + ret = virGetDomain(conn, def->name, def->uuid); def = NULL; /* XM driver owns it now */ } else { if (xenDaemonDomainDefineXML(conn, def) < 0) goto cleanup; + ret = virGetDomain(conn, def->name, def->uuid); } - ret = virGetDomain(conn, def->name, def->uuid); if (ret) ret->id = -1; -- 1.8.1.4

2 2

[libvirt] [PATCH] docs: domain: /dev/urandom isn't a valid rng patch
by Cole Robinson 24 May '13

24 May '13

Only /dev/random and /dev/hwrng are accepted, list them explicitly. --- docs/formatdomain.html.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3a200aa..755d084 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4666,7 +4666,7 @@ qemu-kvm -net nic,model=? /dev/null <dd> <p> This backend type expects a non-blocking character device as input. - Examples of such devices are /dev/random and /dev/urandom. The file + The only accepted paths are /dev/random and /dev/hwrng. The file name is specified as contents of the <code>backend</code> element. When no file name is specified the hypervisor default is used. </p> -- 1.8.2.1

2 2

[libvirt] [PATCH] conf: add missing OOM errors
by Ján Tomko 24 May '13

24 May '13

--- Pushed under the trivial rule. src/conf/virchrdev.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c index 17b1f05..36fbb97 100644 --- a/src/conf/virchrdev.c +++ b/src/conf/virchrdev.c @@ -89,8 +89,10 @@ static char *virChrdevLockFilePath(const char *dev) ++p; } - if (virAsprintf(&path, "%s/LCK..%s", VIR_CHRDEV_LOCK_FILE_PATH, filename) < 0) + if (virAsprintf(&path, "%s/LCK..%s", VIR_CHRDEV_LOCK_FILE_PATH, filename) < 0) { + virReportOOMError(); goto cleanup; + } sanitizedPath = virFileSanitizePath(path); @@ -136,8 +138,10 @@ static int virChrdevLockFileCreate(const char *dev) /* ensure correct format according to filesystem hierarchy standard */ /* http://www.pathname.com/fhs/pub/fhs-2.3.html#VARLOCKLOCKFILES */ - if (virAsprintf(&pidStr, "%10lld\n", (long long) getpid()) < 0) + if (virAsprintf(&pidStr, "%10lld\n", (long long) getpid()) < 0) { + virReportOOMError(); goto cleanup; + } /* create the lock file */ if ((lockfd = open(path, O_WRONLY | O_CREAT | O_EXCL, 00644)) < 0) { -- 1.8.1.5

1 0

[libvirt] [PATCH] qemu: fix NBD migration to hosts with IPv6 enabled
by Ján Tomko 24 May '13

24 May '13

Since f03dcc5 we use [::] as the listening address both on qemu command line in -incoming and in nbd-server-start QMP command. However the latter requires just :: without the braces. --- src/qemu/qemu_migration.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4767908..73ced73 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1112,6 +1112,12 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver, unsigned short port = 0; char *diskAlias = NULL; size_t i; + const char *host; + + if (STREQ(listenAddr, "[::]")) + host = "::"; + else + host = listenAddr; for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDefPtr disk = vm->def->disks[i]; @@ -1133,7 +1139,7 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver, if (!port && ((virPortAllocatorAcquire(driver->remotePorts, &port) < 0) || - (qemuMonitorNBDServerStart(priv->mon, listenAddr, port) < 0))) { + (qemuMonitorNBDServerStart(priv->mon, host, port) < 0))) { qemuDomainObjExitMonitor(driver, vm); goto cleanup; } -- 1.8.1.5

2 2

[libvirt] [PATCH] conf: fix use after free in virChrdevOpen
by Ján Tomko 24 May '13

24 May '13

Don't free the stream on error if we've successfully added it to the hash table, since it will be freed by virChrdevHashEntryFree callback. Preserve the error message before calling virStreamFree, since it resets the error. Reported by Sergey Fionov on libvir-list. --- src/conf/virchrdev.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c index 025d4a8..879c27c 100644 --- a/src/conf/virchrdev.c +++ b/src/conf/virchrdev.c @@ -343,6 +343,8 @@ int virChrdevOpen(virChrdevsPtr devs, virStreamPtr savedStream; const char *path; int ret; + bool added = false; + virErrorPtr savedError; switch (source->type) { case VIR_DOMAIN_CHR_TYPE_PTY: @@ -399,6 +401,7 @@ int virChrdevOpen(virChrdevsPtr devs, if (virHashAddEntry(devs->hash, path, st) < 0) goto error; + added = true; cbdata->devs = devs; if (!(cbdata->path = strdup(path))) { @@ -433,8 +436,16 @@ int virChrdevOpen(virChrdevsPtr devs, return 0; error: - virStreamFree(st); - virHashRemoveEntry(devs->hash, path); + savedError = virSaveLastError(); + + if (added) + virHashRemoveEntry(devs->hash, path); + else + virStreamFree(st); + + virSetError(savedError); + virFreeError(savedError); + if (cbdata) VIR_FREE(cbdata->path); VIR_FREE(cbdata); -- 1.8.1.5

2 2

[libvirt] Possible bug in OpenVZ guest interface name assignment
by ALVARO POLO VALDENEBRO 23 May '13

23 May '13

Hi, I think I've found a possible bug in the way OpenVZ is assigning the guest interface names when network is bridged. It's manifested at least in 1.0.5 version and master branch. As we can see in openvzDomainSetNetwork() function (openvz/openvz_driver.c:822), the field data.ethernet.dev of virDomainNetDefPtr type is used as guest interface name. If not found, then it generates a new one as ethX, calculating the next X value appropriate for the target domain. That's fine for network types other than VIR_DOMAIN_NET_TYPE_BRIDGE. But with bridged networks, there is no chance to populate data.ethernet.dev from the XML descriptor. We could guest that in such circumstance the code will detect that no name is given to data.ethernet.dev, so it will generate a new one. But that's incorrect. The virDomainNetDef.data field is a union, so data.ethernet.dev and data.bridge.brname are located in the same memory offset. That makes the openvzDomainSetNetwork() function to assign the bridge name as guest interface name every time it's invoked for bridged networks. Ideally, for bridged networking openvzDomainSetNetwork() should assign a random ethX device name instead of using the bridge name. Could somebody confirm all this? If so, I could open a new ticket in bugzilla and submit a patch. Best, Alvaro Polo Valdenebro Product Development & Innovation / Telefónica Digital C/ Don Ramón de la Cruz 82-84 Madrid 28006 (+34) 609 087 054 apv(a)tid.es ________________________________ Este mensaje se dirige exclusivamente a su destinatario. Puede consultar nuestra política de envío y recepción de correo electrónico en el enlace situado más abajo. This message is intended exclusively for its addressee. We only send and receive email on the basis of the terms set out at: http://www.tid.es/ES/PAGINAS/disclaimer.aspx

2 1

Re: [libvirt] [virt-tools-list] Q on virsh attach-disk , why no network disk ?
by Cole Robinson 23 May '13

23 May '13

On 05/22/2013 08:55 AM, Deepak C Shetty wrote: > Hi All, > I was looking at virsh attach-disk and i see the below > [snip] > > DESCRIPTION > Attach new disk device. > > OPTIONS > [--domain] <string> domain name, id or uuid > [--source] <string> source of disk device > [--target] <string> target of disk device > ... > ... > > *--sourcetype <string> type of source (block|file)* > [/snip] > > I was wondering why the sourcetype only support block|file only ? What if i > wanted to add a network (disk type=network) based drive ? Is this a constraint > of virsh or QEMU itself doesn't allow hotplu of network block device disk ? > virsh questions go to libvirt-list, CC'd. It's just a missing feature, not a deliberate omission. Patches welcome. - Cole

2 1

[libvirt] [PATCH 1/3] umlConnectTapDevice: initialize tapfd variable
by Michal Privoznik 23 May '13

23 May '13

There is possibility to jump to 'cleanup' label without tapfd variable being initialized. In the label, VIR_FORCE_CLOSE(tapfd) is called which can have fatal consequences. --- src/uml/uml_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index a4088f2..38dcfbb 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -109,7 +109,7 @@ umlConnectTapDevice(virConnectPtr conn, const char *bridge) { bool template_ifname = false; - int tapfd; + int tapfd = -1; if (!net->ifname || STRPREFIX(net->ifname, VIR_NET_GENERATED_PREFIX) || -- 1.8.2.1

2 5

[libvirt] [PATCH v2 00/11] Refactor Xen driver to support ACL work
by Daniel P. Berrange 23 May '13

23 May '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> The Xen driver currently is a really horrible mess, in particular with the way iterates over sub-drivers calling each one in turn, until one magically works. For each operation there is always a clear rule for which sub-driver must be used. Thus instead of blindly iterating over all drivers, we can directly invoke the correct sub-driver. This highlights that quite alot of code is in fact completely unreachable / used & can be deleted. The second big issue is that the Xen sub-drivers will randomly use either the 'id', 'name' or 'uuid' from the virDomainPtr object. If the user provides a malicious virDomainPtr instance, it is possible to have a 'id' referring to domain A, a 'name' referring to domain B and a 'uuid' referring to domain C. This makes doing reliable access control checks difficult. To ensure we have a consistent triple, use the 'uuid' from virDomainPtr to lookup the guest with the hypervisor and return a virDomainDefPtr where only the name/id/uuid are filled out. Use this in the all the subdrivers isolating them from the public virDomainPtr object. The top level Xen driver will then be able to do access control checks on the virDomainDefPtr instances. In v2: - Merged all patches removing the sub-driver iteration concept - Rebased to latest git Daniel P. Berrange (11): Convert Xen domain lookup driver methods to use virDomainDefPtr Convert Xen domain lifecycle driver methods to use virDomainDefPtr Convert Xen domain property driver methods to use virDomainDefPtr Convert Xen domain managed save driver methods to use virDomainDefPtr Convert Xen domain start/migration APIs to use virDomainDefPtr Convert Xen domain VCPU driver methods to use virDomainDefPtr Convert Xen domain device hotplug driver methods to use virDomainDefPtr Convert Xen domain autostart driver methods to use virDomainDefPtr Convert Xen domain scheduler driver methods to use virDomainDefPtr Convert Xen domain stats/peek driver methods to use virDomainDefPtr Convert Xen domain core dump driver methods to use virDomainDefPtr src/conf/domain_conf.c | 24 ++ src/conf/domain_conf.h | 4 + src/libvirt_private.syms | 1 + src/xen/block_stats.c | 6 +- src/xen/block_stats.h | 2 +- src/xen/xen_driver.c | 781 ++++++++++++++++++++++++++++++++++++----------- src/xen/xen_hypervisor.c | 131 ++++---- src/xen/xen_hypervisor.h | 60 ++-- src/xen/xen_inotify.c | 14 +- src/xen/xend_internal.c | 469 ++++++++++++++-------------- src/xen/xend_internal.h | 127 +++++--- src/xen/xm_internal.c | 224 +++++++------- src/xen/xm_internal.h | 71 +++-- 13 files changed, 1230 insertions(+), 684 deletions(-) -- 1.8.1.4

4 29

[libvirt] [PATCHv4 0/4] Multiple TX queue support
by Michal Privoznik 23 May '13

23 May '13

Fixed version with Laine's comments worked in [1]. Moreover, most of the patches joined together. Patches 2, 3 and 5 has been ACKed already. BTW, patch 4 is really a different to patch 3. The only thing they share is a commit message body. Kernel and subsequently QEMU learned multiple transmit queues a while ago. The feature has a nice advantage, it allows a single guest to transmit multiple flows of network data using multiple CPUs simultaneously which increase traffic bandwidth. A lot. The documentation how to use this is available at [2] or [3]. 1: https://www.redhat.com/archives/libvir-list/2013-May/msg01196.html 2: https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/tree/Docume… 3: http://git.qemu.org/?p=qemu.git;a=blob;f=qemu-options.hx;hb=HEAD#l1363 Michal Privoznik (4): Introduce /domain/devices/interface/driver/@queues attribute qemu: Move interface cmd line construction into a separate function qemu: Adapt qemuBuildInterfaceCommandLine to to multiqueue net qemu: Enable multiqueue network docs/formatdomain.html.in | 12 +- docs/schemas/domaincommon.rng | 5 + src/conf/domain_conf.c | 15 + src/conf/domain_conf.h | 1 + src/network/bridge_driver.c | 2 +- src/qemu/qemu_command.c | 462 +++++++++++++-------- src/qemu/qemu_command.h | 13 +- src/qemu/qemu_hotplug.c | 113 +++-- src/qemu/qemu_monitor.c | 78 ++-- src/qemu/qemu_monitor.h | 8 +- src/uml/uml_conf.c | 5 +- src/util/virnetdevtap.c | 113 ++--- src/util/virnetdevtap.h | 2 + .../qemuxml2argvdata/qemuxml2argv-vhost_queues.xml | 51 +++ tests/qemuxml2xmltest.c | 1 + 15 files changed, 593 insertions(+), 288 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-vhost_queues.xml -- 1.8.2.1

3 10

[libvirt] [PATCH] virsh: Fix virDomainFree for NULL domain in blkdeviotune
by Martin Kletzander 23 May '13

23 May '13

Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- Notes: Pushed as 'trivial' tools/virsh-domain.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index bc42408..0402aef 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1130,7 +1130,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) cleanup: virTypedParamsFree(params, nparams); - virDomainFree(dom); + if (dom) + virDomainFree(dom); return ret; save_error: -- 1.8.2.1

1 0

[libvirt] [PATCH] Fix the build failure
by Osier Yang 23 May '13

23 May '13

Commit 16251193afdc forgot to include "virstring.h". Pushed under build-breaker rule. --- src/rpc/virnetservermdns.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rpc/virnetservermdns.c b/src/rpc/virnetservermdns.c index 68d5e4e..4b3c19a 100644 --- a/src/rpc/virnetservermdns.c +++ b/src/rpc/virnetservermdns.c @@ -45,6 +45,7 @@ #include "viralloc.h" #include "virerror.h" #include "virlog.h" +#include "virstring.h" #define VIR_FROM_THIS VIR_FROM_RPC -- 1.8.1.4

1 0

[libvirt] Schedule for next release ?
by Daniel Veillard 23 May '13

23 May '13

It seems that if we want to have the release at the end of the month we ought to enter freeze next Monday the 27 for a release on the 3rd (I may have a hard time making the release on Friday 31 or w.e.). Is there any problem with freezing next Monday ? If not let's proceed that way, in that case 1.0.6 will be out on Monday 3rd June ! 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/

3 2

[libvirt] [PATCH 0/7 v2] Fixes for cgroup setting.
by Osier Yang 23 May '13

23 May '13

/8 ~ 5/8 are refactorings, real fixes are 6/8, 7/8. v1 - v2: * Only rebasing Osier Yang (7): qemu: Abstract the code for blkio controller setting into a helper qemu: Abstract code for memory controller setting into a helper qemu: Abstract code for devices controller setting into a helper qemu: Abstract code for cpuset controller setting into a helper qemu: Abstract code for the cpu controller setting into a helper qemu: Set cpuset.cpus for domain process qemu: Prohibit getting the numa parameters if mode is not strict src/qemu/qemu_cgroup.c | 556 +++++++++++++++++++++++++++++-------------------- src/qemu/qemu_driver.c | 11 +- 2 files changed, 340 insertions(+), 227 deletions(-) -- 1.8.1.4

3 23

[libvirt] [test-API][PATCH] Add env check function for memory_params_live
by Wayne Sun 23 May '13

23 May '13

In this case, domain memory cgroup path is hardcoded and fail the case after cgroup path changed recently. To avoid such failure, add check function for lscgroup command before run this case, if check fail then skip this case. Signed-off-by: Wayne Sun <gsun(a)redhat.com> --- repos/domain/memory_params_live.py | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/repos/domain/memory_params_live.py b/repos/domain/memory_params_live.py index 44fb8b4..280792a 100644 --- a/repos/domain/memory_params_live.py +++ b/repos/domain/memory_params_live.py @@ -4,6 +4,7 @@ import os import math +import commands from xml.dom import minidom import libvirt @@ -15,27 +16,35 @@ required_params = ('guestname', 'hard_limit', 'soft_limit', 'swap_hard_limit', ) optional_params = {} UNLIMITED = 9007199254740991 -CGROUP_PATH = "/cgroup/memory/libvirt/qemu" +CGROUP_PATH = "/cgroup/" def get_cgroup_setting(guestname): """get domain memory parameters in cgroup """ if os.path.exists(CGROUP_PATH): - cgroup_path = "%s/%s" % (CGROUP_PATH, guestname) + cgroup_path = CGROUP_PATH else: - cgroup_path = "/sys/fs%s/%s" % (CGROUP_PATH, guestname) + cgroup_path = "/sys/fs%s" % CGROUP_PATH - f = open("%s/memory.limit_in_bytes" % cgroup_path) + cmd = "lscgroup | grep %s | grep memory:" % guestname + ret, out = commands.getstatusoutput(cmd) + if ret: + logger.error(out) + return 1 + else: + mem_cgroup_path = "%s%s" % (cgroup_path, out.replace(':', '')) + + f = open("%s/memory.limit_in_bytes" % mem_cgroup_path) hard = int(f.read()) logger.info("memory.limit_in_bytes value is %s" % hard) f.close() - f = open("%s/memory.soft_limit_in_bytes" % cgroup_path) + f = open("%s/memory.soft_limit_in_bytes" % mem_cgroup_path) soft = int(f.read()) logger.info("memory.soft_limit_in_bytes value is %s" % soft) f.close() - f = open("%s/memory.memsw.limit_in_bytes" % cgroup_path) + f = open("%s/memory.memsw.limit_in_bytes" % mem_cgroup_path) swap = int(f.read()) logger.info("memory.memsw.limit_in_bytes value is %s" % swap) f.close() @@ -98,6 +107,10 @@ def memory_params_live(params): logger.info("check memory parameters in cgroup") ret = get_cgroup_setting(guestname) + if ret == 1: + logger.error("fail to get domain memory cgroup setting") + return 1 + for i in param_dict.keys(): if math.fabs(param_dict[i] - ret[i]) > 1: logger.error("%s value not match with cgroup setting" % i) @@ -110,3 +123,14 @@ def memory_params_live(params): return 1 return 0 + +def memory_params_live_check(params): + """check lscgroup packages + """ + logger = params['logger'] + cmd = 'lscgroup' + ret, out = commands.getstatusoutput(cmd) + if ret and 'command not found' in out: + logger.error(out) + logger.error("package libcgroup or libcgroup-tools is not installed") + return 1 -- 1.8.2.1

2 2

[libvirt] Remove OPTION section in output of 'virsh help command' if no option exists.
by Zhang Xiaohe 23 May '13

23 May '13

Don't print 'OPTION' if there's no options. Just behaves as DESCRIPTION does. This mostly affects 'interface' command group. Signed-off-by: Zhang Xiaohe <zhangxh(a)cn.fujitsu.com> Reported-by: Li Yang <liyang.fnst(a)cn.fujitsu.com> --- tools/virsh.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index ecb7bd4..7c60800 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1270,7 +1270,9 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname) if (def->opts) { const vshCmdOptDef *opt; - fputs(_("\n OPTIONS\n"), stdout); + /* Print the option only if there are options */ + if (def->opts->name) + fputs(_("\n OPTIONS\n"), stdout); for (opt = def->opts; opt->name; opt++) { switch (opt->type) { case VSH_OT_BOOL: -- 1.7.1

2 2

[libvirt] [PATCH] virNetDevTapCreate: Fail on systems not supporting IFF_MULTI_QUEUE
by Michal Privoznik 23 May '13

23 May '13

In my previous patches I enabled the IFF_MULTI_QUEUE flag every time the user requested multiqueue TAP device. However, this works only at runtime. During build time the flag may be undeclared. --- src/util/virnetdevtap.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index aa41b9c..e8057f7 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -178,8 +178,15 @@ int virNetDevTapCreate(char **ifname, ifr.ifr_flags = IFF_TAP | IFF_NO_PI; /* If tapfdSize is greater than one, request multiqueue */ - if (tapfdSize > 1) + if (tapfdSize > 1) { +# ifdef IFF_MULTI_QUEUE ifr.ifr_flags |= IFF_MULTI_QUEUE; +# else + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Multiqueue devices are not supported on this system")); + goto cleanup; +# endif + } # ifdef IFF_VNET_HDR if ((flags & VIR_NETDEV_TAP_CREATE_VNET_HDR) && -- 1.8.2.1

2 2

[libvirt] [qemu-devel] Default machine type setting for ppc64
by Li Zhang 22 May '13

22 May '13

Hi all, Sorry to bring this problem up again. We encounter this problem in openstack which always use default machine type. Currently, QEMU sets mac99 as default setting for ppc64 but it doesn't work on our platform at all. I tried to fix this in libvirt which it is not acceptable because libvirt only considers to get default setting from QEMU. Maybe I need to cc openstack mailing list, if it is one solution to add one option for machine types it supports. Any suggestion is appreciated. Thanks. :) --Li

7 21

[libvirt] [PATCH 0/3 v4] add 'sharePolicy' attribute for qemu vnc sharing policy
by Guannan Ren 22 May '13

22 May '13

v3->v4 add missing .args, .xml files (I raw it in git Untracked files this morning and thought about it for a while, then git-cleaned them...) rebase work. v2->v3 rebase work. v1->v2: changed attribute name from 'policy' to 'sharePolicy' renamed caps flag name: QEMU_CAPS_VNC_SHARE_POLICY fixed issues pointed out in v1 review have to keep hard-coded version probe after checking qemu -help and qmp command. As there were some vnc patches recently, I chose do rebase work a little later. These patches try to add a new attribute 'sharePolicy' to element graphics of vnc type. This attribute has three values: allow-exclusive,force-shared and ignore.

2 5

[libvirt] [PATCH] vbox: fix VIR_STRDUP value check
by Ján Tomko 22 May '13

22 May '13

In my review of 31532ca I missed the fact that VIR_STRDUP now returns 1 on success, and 0 if the source was NULL. (This still doesn't add proper OOM error handling.) --- src/vbox/vbox_tmpl.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index eb8ac63..163aeff 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -2688,9 +2688,9 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { if (hddType == HardDiskType_Immutable) def->disks[hddNum]->readonly = true; - if (VIR_STRDUP_QUIET(def->disks[hddNum]->src, hddlocation) == 0 && - VIR_STRDUP_QUIET(def->disks[hddNum]->dst, "hdd") == 0) - hddNum++; + ignore_value(VIR_STRDUP(def->disks[hddNum]->src, hddlocation)); + ignore_value(VIR_STRDUP(def->disks[hddNum]->dst, "hdd")); + hddNum++; VBOX_UTF8_FREE(hddlocation); VBOX_UTF16_FREE(hddlocationUtf16); @@ -7507,7 +7507,7 @@ static int vboxConnectListNetworks(virConnectPtr conn, char **const names, int n VBOX_UTF16_TO_UTF8(nameUtf16, &nameUtf8); VIR_DEBUG("nnames[%d]: %s", ret, nameUtf8); - if (VIR_STRDUP(names[ret], nameUtf8) == 0) + if (VIR_STRDUP(names[ret], nameUtf8) >= 0) ret++; VBOX_UTF8_FREE(nameUtf8); @@ -7585,7 +7585,7 @@ static int vboxConnectListDefinedNetworks(virConnectPtr conn, char **const names VBOX_UTF16_TO_UTF8(nameUtf16, &nameUtf8); VIR_DEBUG("nnames[%d]: %s", ret, nameUtf8); - if (VIR_STRDUP(names[ret], nameUtf8) == 0) + if (VIR_STRDUP(names[ret], nameUtf8) >= 0) ret++; VBOX_UTF8_FREE(nameUtf8); @@ -8087,7 +8087,7 @@ static char *vboxNetworkGetXMLDesc(virNetworkPtr network, networkInterface->vtbl->GetInterfaceType(networkInterface, &interfaceType); if (interfaceType == HostNetworkInterfaceType_HostOnly) { - if (VIR_STRDUP(def->name, network->name) == 0) { + if (VIR_STRDUP(def->name, network->name) >= 0) { PRUnichar *networkNameUtf16 = NULL; IDHCPServer *dhcpServer = NULL; vboxIID vboxnet0IID = VBOX_IID_INITIALIZER; -- 1.8.1.5

3 4

[libvirt] [PATCH 00/31] Forbid whitespace before ";"
by Osier Yang 22 May '13

22 May '13

Inspired by [1] from Daniel, when I tried to add the syntax-check rule, it detected the various places which use the whitespace before ";". So before adding the syntax-check rule, I cleaned up them first by a simple Bash script: \#!/bin/bash for dir in $dirs; do files=$(find $dir -type f -name "*.[ch]") for file in $files; do sed -r -i -e "/^ \s+;/!s/\s+;/;/g" $file done git add $files git commit -m "$1: Remove the whitespace before ';'" done So it's rather mechanically, except I need to change a few places of the generated patch. [1] https://www.redhat.com/archives/libvir-list/2013-May/msg01397.html Osier Yang (31): src/network: Remove the whitespace before ';' src/libxl: Remove the whitespace before ';' src/conf: Remove the whitespace before ';' src/test: Remove the whitespace before ';' src/vbox: Remove the whitespace before ';' src/xenxs: Remove the whitespace before ';' src/nwfilter: Remove the whitespace before ';' src/phyp: Remove the whitespace before ';' src/node_device: Remove the whitespace before ';' src/openvz: Remove the whitespace before ';' src/uml: Remove the whitespace before ';' src/parallels: Remove the whitespace before ';' src/interface: Remove the whitespace before ';' src/qemu: Remove the whitespace before ';' src/vmware: Remove the whitespace before ';' src/xen: Remove the whitespace before ';' src/security: Remove the whitespace before ';' src/storage: Remove the whitespace before ';' tools: Remove the whitespace before ";" daemon: Remove the whitespace before ";" tests/: Remove the whitespace before ";" src/utils: Remove the whitespace before ";" src/rpc: Remove the whitespace before ";" src/remote: Remove the whitespace before ";" src/lxc: Remove the whitespace before ";" examples: Remove the whitespace before ';' python: Remove the whitespace before ";" src/locking: Remove the whitespace before ";" src/*.[ch]: Remove the whitespace before ";" nwfitler: Change the comment style syntax-check: Add the rule to forbid whitespace before ";" build-aux/bracket-spacing.pl | 18 +++ daemon/libvirtd-config.c | 2 +- daemon/remote.c | 12 +- examples/domain-events/events-c/event-test.c | 2 +- examples/openauth/openauth.c | 4 +- python/libvirt-lxc-override.c | 4 +- python/libvirt-override.c | 16 +- src/conf/capabilities.c | 50 +++--- src/conf/cpu_conf.c | 14 +- src/conf/domain_audit.c | 10 +- src/conf/domain_conf.c | 230 +++++++++++++-------------- src/conf/domain_event.c | 32 ++-- src/conf/interface_conf.c | 8 +- src/conf/network_conf.c | 28 ++-- src/conf/node_device_conf.c | 12 +- src/conf/nwfilter_conf.c | 8 +- src/conf/storage_conf.c | 8 +- src/interface/interface_backend_udev.c | 2 +- src/libvirt-lxc.c | 2 +- src/libvirt.c | 10 +- src/libxl/libxl_conf.c | 4 +- src/libxl/libxl_driver.c | 2 +- src/locking/domain_lock.c | 4 +- src/locking/lock_daemon.c | 4 +- src/locking/lock_driver_lockd.c | 12 +- src/locking/lock_driver_sanlock.c | 6 +- src/locking/lock_manager.c | 2 +- src/lxc/lxc_cgroup.c | 4 +- src/lxc/lxc_container.c | 24 +-- src/lxc/lxc_controller.c | 28 ++-- src/lxc/lxc_driver.c | 4 +- src/lxc/lxc_hostdev.c | 4 +- src/lxc/lxc_process.c | 18 +-- src/network/bridge_driver.c | 22 +-- src/node_device/node_device_udev.c | 2 +- src/nodeinfo.c | 18 +-- src/nwfilter/nwfilter_dhcpsnoop.c | 10 +- src/nwfilter/nwfilter_driver.c | 6 +- src/openvz/openvz_driver.c | 6 +- src/parallels/parallels_network.c | 12 +- src/phyp/phyp_driver.c | 6 +- src/qemu/qemu_agent.c | 4 +- src/qemu/qemu_capabilities.c | 32 ++-- src/qemu/qemu_cgroup.c | 4 +- src/qemu/qemu_command.c | 170 ++++++++++---------- src/qemu/qemu_conf.c | 4 +- src/qemu/qemu_domain.c | 12 +- src/qemu/qemu_driver.c | 30 ++-- src/qemu/qemu_hostdev.c | 8 +- src/qemu/qemu_hotplug.c | 14 +- src/qemu/qemu_migration.c | 6 +- src/qemu/qemu_monitor.c | 2 +- src/qemu/qemu_monitor_json.c | 34 ++-- src/qemu/qemu_monitor_text.c | 2 +- src/qemu/qemu_process.c | 44 ++--- src/remote/remote_driver.c | 24 +-- src/rpc/virnetclient.c | 12 +- src/rpc/virnetclientprogram.c | 12 +- src/rpc/virnetmessage.c | 6 +- src/rpc/virnetserver.c | 26 +-- src/rpc/virnetserverclient.c | 4 +- src/rpc/virnetserverprogram.c | 2 +- src/rpc/virnetserverservice.c | 14 +- src/rpc/virnetsocket.c | 2 +- src/rpc/virnettlscontext.c | 2 +- src/security/security_dac.c | 8 +- src/security/security_driver.c | 2 +- src/security/security_selinux.c | 8 +- src/security/virt-aa-helper.c | 4 +- src/storage/storage_backend.c | 14 +- src/storage/storage_backend_disk.c | 2 +- src/storage/storage_backend_iscsi.c | 10 +- src/storage/storage_backend_logical.c | 6 +- src/storage/storage_driver.c | 26 +-- src/test/test_driver.c | 80 +++++----- src/uml/uml_conf.c | 12 +- src/uml/uml_driver.c | 14 +- src/util/virarch.c | 2 +- src/util/virauth.c | 2 +- src/util/vircgroup.c | 38 ++--- src/util/vircommand.c | 12 +- src/util/vireventpoll.c | 22 +-- src/util/virhash.c | 6 +- src/util/viridentity.c | 4 +- src/util/virjson.c | 16 +- src/util/virlockspace.c | 10 +- src/util/virportallocator.c | 2 +- src/util/virprocess.c | 14 +- src/util/virstoragefile.c | 4 +- src/util/virstring.c | 2 +- src/util/virthreadwin32.c | 4 +- src/util/viruri.c | 2 +- src/util/virutil.c | 2 +- src/vbox/vbox_tmpl.c | 2 +- src/vmware/vmware_conf.c | 2 +- src/vmware/vmware_driver.c | 2 +- src/xen/xen_driver.c | 6 +- src/xen/xen_driver.h | 2 +- src/xen/xen_hypervisor.c | 4 +- src/xen/xen_inotify.c | 4 +- src/xen/xm_internal.c | 4 +- src/xen/xs_internal.c | 12 +- src/xenxs/xen_sxpr.c | 20 +-- src/xenxs/xen_xm.c | 16 +- tests/commandhelper.c | 6 +- tests/eventtest.c | 18 +-- tests/fdstreamtest.c | 14 +- tests/libvirtdconftest.c | 2 +- tests/nodeinfotest.c | 2 +- tests/qemuhelptest.c | 2 +- tests/qemumonitortestutils.c | 2 +- tests/qemuxml2argvtest.c | 4 +- tests/securityselinuxlabeltest.c | 12 +- tests/ssh.c | 4 +- tests/testutils.c | 16 +- tests/vircgrouptest.c | 2 +- tests/virnetsockettest.c | 10 +- tests/viruritest.c | 2 +- tools/virsh-domain.c | 6 +- tools/virsh-nodedev.c | 6 +- tools/virsh-nwfilter.c | 2 +- tools/virsh-secret.c | 2 +- tools/virsh.c | 4 +- 123 files changed, 838 insertions(+), 820 deletions(-) -- 1.8.1.4

3 42

[libvirt] [PATCH 1/3] Fix the syntax-check failure
by Osier Yang 22 May '13

22 May '13

Introduced by commit 7ac2c4fe624, pushed under build-breaker rule. --- src/conf/interface_conf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h index ae93811..ed6986c 100644 --- a/src/conf/interface_conf.h +++ b/src/conf/interface_conf.h @@ -211,7 +211,7 @@ char *virInterfaceDefFormat(const virInterfaceDefPtr def); void virInterfaceObjLock(virInterfaceObjPtr obj); void virInterfaceObjUnlock(virInterfaceObjPtr obj); -#define VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE \ +# define VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE \ (VIR_CONNECT_LIST_INTERFACES_ACTIVE | \ VIR_CONNECT_LIST_INTERFACES_INACTIVE) -- 1.8.1.4

1 0

[libvirt] [PATCH v2] interface: list all interfaces with flags == 0
by Guannan Ren 22 May '13

22 May '13

virConnectListAllInterfaces should support to list all of interfaces when the value of flags is 0. The behaviour is consistent with other virConnectListAll* APIs --- src/conf/interface_conf.h | 4 ++++ src/interface/interface_backend_netcf.c | 29 +++++++++++++++-------------- src/interface/interface_backend_udev.c | 25 ++++++++++++------------- 3 files changed, 31 insertions(+), 27 deletions(-) diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h index e636c35..ae93811 100644 --- a/src/conf/interface_conf.h +++ b/src/conf/interface_conf.h @@ -211,4 +211,8 @@ char *virInterfaceDefFormat(const virInterfaceDefPtr def); void virInterfaceObjLock(virInterfaceObjPtr obj); void virInterfaceObjUnlock(virInterfaceObjPtr obj); +#define VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE \ + (VIR_CONNECT_LIST_INTERFACES_ACTIVE | \ + VIR_CONNECT_LIST_INTERFACES_INACTIVE) + #endif /* __INTERFACE_CONF_H__ */ diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c index cbba4fd..d626017 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -260,6 +260,7 @@ static int netcfConnectListDefinedInterfaces(virConnectPtr conn, char **const na } +#define MATCH(FLAG) (flags & (FLAG)) static int netcfConnectListAllInterfaces(virConnectPtr conn, virInterfacePtr **ifaces, @@ -276,8 +277,7 @@ netcfConnectListAllInterfaces(virConnectPtr conn, int ret = -1; char **names = NULL; - virCheckFlags(VIR_CONNECT_LIST_INTERFACES_ACTIVE | - VIR_CONNECT_LIST_INTERFACES_INACTIVE, -1); + virCheckFlags(VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE, -1); interfaceDriverLock(driver); @@ -293,7 +293,6 @@ netcfConnectListAllInterfaces(virConnectPtr conn, _("failed to get number of host interfaces: %s%s%s"), errmsg, details ? " - " : "", details ? details : ""); - ret = -1; goto cleanup; } @@ -304,7 +303,6 @@ netcfConnectListAllInterfaces(virConnectPtr conn, if (VIR_ALLOC_N(names, count) < 0) { virReportOOMError(); - ret = -1; goto cleanup; } @@ -361,16 +359,19 @@ netcfConnectListAllInterfaces(virConnectPtr conn, /* XXX: Filter the result, need to be splitted once new filter flags * except active|inactive are supported. */ - if (((status & NETCF_IFACE_ACTIVE) && - (flags & VIR_CONNECT_LIST_INTERFACES_ACTIVE)) || - ((status & NETCF_IFACE_INACTIVE) && - (flags & VIR_CONNECT_LIST_INTERFACES_INACTIVE))) { - if (ifaces) { - iface_obj = virGetInterface(conn, ncf_if_name(iface), - ncf_if_mac_string(iface)); - tmp_iface_objs[niface_objs] = iface_obj; - } - niface_objs++; + if (MATCH(VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE) && + !((MATCH(VIR_CONNECT_LIST_INTERFACES_ACTIVE) && + (status & NETCF_IFACE_ACTIVE)) || + (MATCH(VIR_CONNECT_LIST_INTERFACES_INACTIVE) && + (status & NETCF_IFACE_INACTIVE)))) { + ncf_if_free(iface); + continue; + } + + if (ifaces) { + iface_obj = virGetInterface(conn, ncf_if_name(iface), + ncf_if_mac_string(iface)); + tmp_iface_objs[niface_objs++] = iface_obj; } ncf_if_free(iface); diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c index 1fd7d46..428adc8 100644 --- a/src/interface/interface_backend_udev.c +++ b/src/interface/interface_backend_udev.c @@ -282,6 +282,7 @@ udevConnectListDefinedInterfaces(virConnectPtr conn, VIR_UDEV_IFACE_INACTIVE); } +#define MATCH(FLAG) (flags & (FLAG)) static int udevConnectListAllInterfaces(virConnectPtr conn, virInterfacePtr **ifaces, @@ -299,8 +300,7 @@ udevConnectListAllInterfaces(virConnectPtr conn, int status = 0; int ret; - virCheckFlags(VIR_CONNECT_LIST_INTERFACES_ACTIVE | - VIR_CONNECT_LIST_INTERFACES_INACTIVE, -1); + virCheckFlags(VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE, -1); /* Grab a udev reference */ udev = udev_ref(driverState->udev); @@ -354,7 +354,6 @@ udevConnectListAllInterfaces(virConnectPtr conn, const char *path; const char *name; const char *macaddr; - int add_to_list = 0; path = udev_list_entry_get_name(dev_entry); dev = udev_device_new_from_syspath(udev, path); @@ -363,18 +362,17 @@ udevConnectListAllInterfaces(virConnectPtr conn, status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up"); /* Filter the results */ - if (status && (flags & VIR_CONNECT_LIST_INTERFACES_ACTIVE)) - add_to_list = 1; - else if (!status && (flags & VIR_CONNECT_LIST_INTERFACES_INACTIVE)) - add_to_list = 1; + if (MATCH(VIR_CONNECT_LIST_INTERFACES_FILTERS_ACTIVE) && + !((MATCH(VIR_CONNECT_LIST_INTERFACES_ACTIVE) && status) || + (MATCH(VIR_CONNECT_LIST_INTERFACES_INACTIVE) && !status))) { + udev_device_unref(dev); + continue; + } /* If we matched a filter, then add it */ - if (add_to_list) { - if (ifaces) { - iface_obj = virGetInterface(conn, name, macaddr); - ifaces_list[count] = iface_obj; - } - count++; + if (ifaces) { + iface_obj = virGetInterface(conn, name, macaddr); + ifaces_list[count++] = iface_obj; } udev_device_unref(dev); } @@ -387,6 +385,7 @@ udevConnectListAllInterfaces(virConnectPtr conn, if (ifaces) { ignore_value(VIR_REALLOC_N(ifaces_list, count + 1)); *ifaces = ifaces_list; + ifaces_list = NULL; } return count; -- 1.8.1.4

2 2

[libvirt] [PATCH] cgroup: be robust against cgroup movement races
by Eric Blake 22 May '13

22 May '13

https://bugzilla.redhat.com/show_bug.cgi?id=965169 documents a problem starting domains when cgroups are enabled; I was able to reliably reproduce the race about 5% of the time when I added hooks to domain startup by 3 seconds (as that seemed to be about the length of time that qemu created and then closed a temporary thread, probably related to aio handling of initially opening a disk image). There are some inherent TOCTTOU races when moving tasks between kernel cgroups, precisely because threads can be created or completed in the window between when we read a thread id from the source and when we write to the destination. As the goal of virCgroupMoveTask is merely to move ALL tasks into the new cgroup, it is sufficient to iterate until no more threads are being created in the old group, and ignoring any threads that die before we can move them. It would be nicer to start the threads in the right cgroup to begin with, but by default, all child threads are created in the same cgroup as their parent, and we don't want vcpu child threads in the emulator cgroup, so I don't see any good way of avoiding the move. It would also be nice if the kernel were to implement something like rename() as a way to atomically move a group of threads from one cgroup to another, instead of forcing a window where we have to read and parse the source, then format and write back into the destination. * src/util/vircgroup.c (virCgroupAddTaskStrController): Ignore ESRCH, because a thread ended between read and write attempts. (virCgroupMoveTask): Loop until all threads have moved. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- src/util/vircgroup.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 07ea2c3..6d44694 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1037,7 +1037,11 @@ static int virCgroupAddTaskStrController(virCgroupPtr group, goto cleanup; rc = virCgroupAddTaskController(group, p, controller); - if (rc != 0) + /* A thread that exits between when we first read the source + * tasks and now is not fatal. */ + if (rc == -ESRCH) + rc = 0; + else if (rc != 0) goto cleanup; next = strchr(cur, '\n'); @@ -1074,15 +1078,23 @@ int virCgroupMoveTask(virCgroupPtr src_group, virCgroupPtr dest_group) !dest_group->controllers[i].mountPoint) continue; - rc = virCgroupGetValueStr(src_group, i, "tasks", &content); - if (rc != 0) - return rc; + /* New threads are created in the same group as their parent; + * but if a thread is created after we first read we aren't + * aware that it needs to move. Therefore, we must iterate + * until content is empty. */ + while (1) { + rc = virCgroupGetValueStr(src_group, i, "tasks", &content); + if (rc != 0) + return rc; + if (!*content) + break; - rc = virCgroupAddTaskStrController(dest_group, content, i); - if (rc != 0) - goto cleanup; + rc = virCgroupAddTaskStrController(dest_group, content, i); + if (rc != 0) + goto cleanup; - VIR_FREE(content); + VIR_FREE(content); + } } cleanup: -- 1.8.1.4

2 2