November 2009 - Devel - libvirt List Archives

[libvirt] PATCH: recent qemu-kvm requires -enable-kvm flag to activate VT optimization
by Steve Yarmie 20 Nov '09

20 Nov '09

Hello, Recent qemu releases require command option '-enable-qemu' in order for the kvm functionality be activated. Libvirt needs to pass this flag to qemu when starting a domain. Note that without the option, even if both the kernel and qemu support KVM, KVM will not be activated and VMs will be slow as molasses ;) I added a check in qemu_conf.c to see if `qemu -help` emits the -enable-kvm flag, and if so, use the flag accordingly. There exists a check in src/qemu/qemu_conf.c to check for the -no-kvm flag. This check works the same way. Adding the included code will not break systems where qemu does not have the option. See 'qemu -help': ... -enable-kvm enable KVM full virtualization support ... included patch against src/qemu/qemu_conf.{c,h} -Steve diff -ur libvirt/src/qemu/qemu_conf.c libvirt-qemu-enable-kvm/src/qemu/qemu_conf.c --- libvirt/src/qemu/qemu_conf.c 2009-11-08 20:57:08.438908134 -0800 +++ libvirt-qemu-enable-kvm/src/qemu/qemu_conf.c 2009-11-08 22:36:04.299921427 -0800 @@ -1602,6 +1602,7 @@ struct utsname ut; int disableKQEMU = 0; int disableKVM = 0; + int enableKVM = 0; int qargc = 0, qarga = 0; const char **qargv = NULL; int qenvc = 0, qenva = 0; @@ -1660,6 +1661,15 @@ def->virtType == VIR_DOMAIN_VIRT_QEMU) disableKVM = 1; + /* Should explicitly enable KVM if + * 1. Guest domain is 'qemu' + * 2. The qemu binary has the -enable-kvm flag + * NOTE: user must be responsible to load the kvm modules + */ + if ((qemuCmdFlags & QEMUD_CMD_FLAG_ENABLE_KVM) && + def->virtType == VIR_DOMAIN_VIRT_QEMU) + enableKVM = 1; + /* * Need to force a 32-bit guest CPU type if * @@ -1787,6 +1797,8 @@ ADD_ARG_LIT("-no-kqemu"); if (disableKVM) ADD_ARG_LIT("-no-kvm"); + if (enableKVM) + ADD_ARG_LIT("-enable-kvm"); ADD_ARG_LIT("-m"); ADD_ARG_LIT(memory); if (def->hugepage_backed) { diff -ur libvirt/src/qemu/qemu_conf.h libvirt-qemu-enable-kvm/src/qemu/qemu_conf.h --- libvirt/src/qemu/qemu_conf.h 2009-11-08 20:57:08.438908134 -0800 +++ libvirt-qemu-enable-kvm/src/qemu/qemu_conf.h 2009-11-08 14:57:39.000000000 -0800 @@ -73,6 +73,7 @@ QEMUD_CMD_FLAG_XEN_DOMID = (1 << 20), /* -xen-domid (new style xen integration) */ QEMUD_CMD_FLAG_MIGRATE_QEMU_UNIX = (1 << 21), /* Does qemu support unix domain sockets for migration? */ QEMUD_CMD_FLAG_CHARDEV = (1 << 22), /* Is the new -chardev arg available */ + QEMUD_CMD_FLAG_ENABLE_KVM = (1 << 23), /* Is the -enable-kvm flag available to "enable KVM full virtualization support" */ }; /* Main driver state */

3 7

[libvirt] PATCH: iptables.c: fix deprecated cmd syntax
by Steve Yarmie 20 Nov '09

20 Nov '09

default iptables rules uses deprecated cmdline syntax When using iptables-1.4.5 with libvirt warnings appear: debug : virRun:827 : Command stderr: Using intrapositioned negation (`--option ! this`) is deprecated in favor of extrapositioned (`! --option this`). simple patch against src/util/iptables.c fixes it.. Reference Bugzilla ID: 533775 -Steve diff -ur libvirt/src/util/iptables.c libvirt-ipt/src/util/iptables.c --- libvirt/src/util/iptables.c 2009-11-08 20:57:08.458909225 -0800 +++ libvirt-ipt/src/util/iptables.c 2009-11-08 21:19:48.554615398 -0800 @@ -1067,7 +1067,7 @@ return iptablesAddRemoveRule(ctx->nat_postrouting, action, "--source", network, - "--destination", "!", network, + "!", "--destination", network, "--out-interface", physdev, "--jump", "MASQUERADE", NULL); @@ -1075,7 +1075,7 @@ return iptablesAddRemoveRule(ctx->nat_postrouting, action, "--source", network, - "--destination", "!", network, + "!", "--destination", network, "--jump", "MASQUERADE", NULL); }

3 2

[libvirt] [PATCH] remove sysfs_path and parent_sysfs_path from XML
by Dave Allan 20 Nov '09

20 Nov '09

I erroneously included the sysfs_path and parent_sysfs_path elements in the node device xml. The attached patch removes them. Dave >From d848b0c5d9c92546e95c081eb7ca185101d71507 Mon Sep 17 00:00:00 2001 From: David Allan <dallan(a)redhat.com> Date: Wed, 18 Nov 2009 17:29:35 -0500 Subject: [PATCH 1/1] Remove sysfs_path and parent_sysfs_path from XML * These two elements were included erroneously in the udev backend commit --- src/conf/node_device_conf.c | 8 -------- 1 files changed, 0 insertions(+), 8 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index add9d2b..4a15644 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -251,17 +251,9 @@ char *virNodeDeviceDefFormat(virConnectPtr conn, virBufferAddLit(&buf, "<device>\n"); virBufferEscapeString(&buf, " <name>%s</name>\n", def->name); - if (def->sysfs_path != NULL) { - virBufferEscapeString(&buf, " <sysfs_path>%s</sysfs_path>\n", - def->sysfs_path); - } if (def->parent) { virBufferEscapeString(&buf, " <parent>%s</parent>\n", def->parent); } - if (def->parent_sysfs_path != NULL) { - virBufferEscapeString(&buf, " <parent_sysfs_path>%s</parent_sysfs_path>\n", - def->parent_sysfs_path); - } if (def->driver) { virBufferAddLit(&buf, " <driver>\n"); virBufferEscapeString(&buf, " <name>%s</name>\n", def->driver); -- 1.6.5.2

3 2

[libvirt] [PATCH] make qemu monitor timeout configurable
by Dan Kenigsberg 20 Nov '09

20 Nov '09

we've experience cases where waiting 30 seconds for a qemu monitor to finish was not enough (slow and busy server with slow storage). that was with managing qemu processes directly, btw, not with libvirt. however those case where not common. how about making the timeout configurable on-site? --- src/qemu/qemu.conf | 5 +++++ src/qemu/qemu_conf.c | 10 ++++++++++ src/qemu/qemu_conf.h | 2 ++ src/qemu/qemu_driver.c | 29 ++++++++++++++--------------- 4 files changed, 31 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 2129bae..bca858a 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -157,3 +157,8 @@ # This currently requires ebtables to be installed. # # mac_filter = 1 + +# job_wait_time specifies how many seconds should a qemu monitor query wait for +# the monitor mutex. +# +# job_wait_time = 30 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 225a760..bcbe80d 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -340,6 +340,16 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, } } + p = virConfGetValue (conf, "job_wait_time"); + CHECK_TYPE ("job_wait_time", VIR_CONF_LONG); + if (p && p->l) { + driver->job_wait_time = 1000ull * p->l; + } else { + driver->job_wait_time = 1000ull * 30; + } + if (driver->job_wait_time <= 0) + driver->job_wait_time = 1; + virConfFree (conf); return 0; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 1931aef..0d816e2 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -134,6 +134,8 @@ struct qemud_driver { char *saveImageFormat; pciDeviceList *activePciHostdevs; + + int job_wait_time; }; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a4a87ac..2f273eb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -160,11 +160,10 @@ static void qemuDomainObjPrivateFree(void *data) * successful calls must be followed by EndJob eventually */ -/* Give up waiting for mutex after 30 seconds */ -#define QEMU_JOB_WAIT_TIME (1000ull * 30) - -static int qemuDomainObjBeginJob(virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK; -static int qemuDomainObjBeginJob(virDomainObjPtr obj) +static int qemuDomainObjBeginJob(struct qemud_driver *driver, + virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK; +static int qemuDomainObjBeginJob(struct qemud_driver *driver, + virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; struct timeval now; @@ -176,7 +175,7 @@ static int qemuDomainObjBeginJob(virDomainObjPtr obj) return -1; } then = (now.tv_sec * 1000ull) + (now.tv_usec / 1000); - then += QEMU_JOB_WAIT_TIME; + then += driver->job_wait_time; virDomainObjRef(obj); @@ -218,7 +217,7 @@ static int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver, return -1; } then = (now.tv_sec * 1000ull) + (now.tv_usec / 1000); - then += QEMU_JOB_WAIT_TIME; + then += driver->job_wait_time; virDomainObjRef(obj); qemuDriverUnlock(driver); @@ -3022,7 +3021,7 @@ static int qemudDomainShutdown(virDomainPtr dom) { goto cleanup; } - if (qemuDomainObjBeginJob(vm) < 0) + if (qemuDomainObjBeginJob(driver, vm) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { @@ -3203,7 +3202,7 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) { goto cleanup; } - if (qemuDomainObjBeginJob(vm) < 0) + if (qemuDomainObjBeginJob(driver, vm) < 0) goto cleanup; if (virDomainObjIsActive(vm)) { @@ -3269,7 +3268,7 @@ static int qemudDomainGetInfo(virDomainPtr dom, if (virDomainObjIsActive(vm)) { qemuDomainObjPrivatePtr priv = vm->privateData; if (!priv->jobActive) { - if (qemuDomainObjBeginJob(vm) < 0) + if (qemuDomainObjBeginJob(driver, vm) < 0) goto cleanup; qemuDomainObjEnterMonitor(vm); @@ -3544,7 +3543,7 @@ static int qemudDomainCoreDump(virDomainPtr dom, goto cleanup; } - if (qemuDomainObjBeginJob(vm) < 0) + if (qemuDomainObjBeginJob(driver, vm) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { @@ -3618,7 +3617,7 @@ static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { goto cleanup; } - if (qemuDomainObjBeginJob(vm) < 0) + if (qemuDomainObjBeginJob(driver, vm) < 0) goto cleanup; if (virDomainObjIsActive(vm)) { @@ -4145,7 +4144,7 @@ static char *qemudDomainDumpXML(virDomainPtr dom, /* Don't delay if someone's using the monitor, just use * existing most recent data instead */ if (!priv->jobActive) { - if (qemuDomainObjBeginJob(vm) < 0) + if (qemuDomainObjBeginJob(driver, vm) < 0) goto cleanup; qemuDomainObjEnterMonitor(vm); @@ -5777,7 +5776,7 @@ qemudDomainBlockStats (virDomainPtr dom, goto cleanup; } - if (qemuDomainObjBeginJob(vm) < 0) + if (qemuDomainObjBeginJob(driver, vm) < 0) goto cleanup; if (!virDomainObjIsActive (vm)) { @@ -5986,7 +5985,7 @@ qemudDomainMemoryPeek (virDomainPtr dom, goto cleanup; } - if (qemuDomainObjBeginJob(vm) < 0) + if (qemuDomainObjBeginJob(driver, vm) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { -- 1.6.5.2

2 1

[libvirt] [PATCH] Removing devicePath member from dev struct
by Dave Allan 20 Nov '09

20 Nov '09

I realized that I inadvertently added a member to the def struct to contain each device's sysfs path when there was an existing member in the dev struct for "OS specific path to device metadat, eg sysfs" Since the udev backend needs to record the sysfs path while it's in the process of creating the device, before the dev struct gets allocated, I chose to remove the member from the dev struct. I've attached a patch. An alternative would be to store the information twice, but that seems crufty to me. Dave >From 74b97811c298653924b10ce29c26e4282c3786a3 Mon Sep 17 00:00:00 2001 From: David Allan <dallan(a)redhat.com> Date: Wed, 18 Nov 2009 16:10:32 -0500 Subject: [PATCH 1/1] Remove devicePath member from dev struct to avoid redundancy --- src/conf/node_device_conf.c | 1 - src/conf/node_device_conf.h | 1 - src/node_device/node_device_driver.c | 2 +- src/node_device/node_device_hal.c | 2 +- src/node_device/node_device_udev.c | 7 ------- 5 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index f55c9c7..f2faeec 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -156,7 +156,6 @@ void virNodeDeviceObjFree(virNodeDeviceObjPtr dev) if (!dev) return; - VIR_FREE(dev->devicePath); virNodeDeviceDefFree(dev->def); if (dev->privateFree) (*dev->privateFree)(dev->privateData); diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 639a7e7..7a20bd6 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -178,7 +178,6 @@ typedef virNodeDeviceObj *virNodeDeviceObjPtr; struct _virNodeDeviceObj { virMutex lock; - char *devicePath; /* OS specific path to device metadat, eg sysfs */ virNodeDeviceDefPtr def; /* device definition */ void *privateData; /* driver-specific private data */ void (*privateFree)(void *data); /* destructor for private data */ diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index cddd994..7aed916 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -85,7 +85,7 @@ static int update_driver_name(virConnectPtr conn, VIR_FREE(dev->def->driver); - if (virAsprintf(&driver_link, "%s/driver", dev->devicePath) < 0) { + if (virAsprintf(&driver_link, "%s/driver", dev->def->sysfs_path) < 0) { virReportOOMError(conn); goto cleanup; } diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 1e1d872..31c1764 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -470,7 +470,7 @@ static void dev_create(const char *udi) dev->privateData = privData; dev->privateFree = free_udi; - dev->devicePath = devicePath; + dev->def->sysfs_path = devicePath; virNodeDeviceObjUnlock(dev); diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 4ddf360..9b48052 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1230,13 +1230,6 @@ static int udevAddOneDevice(struct udev_device *device) goto out; } - dev->devicePath = strdup(udev_device_get_devpath(device)); - if (dev->devicePath == NULL) { - virReportOOMError(NULL); - virNodeDeviceObjRemove(&driverState->devs, dev); - goto out; - } - virNodeDeviceObjUnlock(dev); ret = 0; -- 1.6.5.2

2 2

[libvirt] report OOM in two places in node_device_driver.c
by Dave Allan 19 Nov '09

19 Nov '09

I noticed that there are two places in node_device_driver.c in which we don't call virReportOOMError after strdup failure. The attached small patch adds the two calls. Dave diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index cddd994..b474d43 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -175,6 +175,7 @@ nodeListDevices(virConnectPtr conn, virNodeDeviceHasCap(driver->devs.objs[i], cap)) { if ((names[ndevs++] = strdup(driver->devs.objs[i]->def->name)) == NULL) { virNodeDeviceObjUnlock(driver->devs.objs[i]); + virReportOOMError(conn); goto failure; } } @@ -379,8 +380,10 @@ nodeDeviceListCaps(virNodeDevicePtr dev, char **const names, int maxnames) for (caps = obj->def->caps; caps && ncaps < maxnames; caps = caps->next) { names[ncaps] = strdup(virNodeDevCapTypeToString(caps->type)); - if (names[ncaps++] == NULL) + if (names[ncaps++] == NULL) { + virReportOOMError(dev->conn); goto cleanup; + } } ret = ncaps;

2 2

[libvirt] [PATCH] Fix labelling on QEMU restore images
by Daniel P. Berrange 19 Nov '09

19 Nov '09

Even though QEMU does not directly open the saved image when restoring, it must be correctly labelled to allow QEMU to read from it because labelling is passed around with open file descriptors. The labelling should not allow writing to the saved image again, only reading. * src/qemu/qemu_driver.c: Label the save image when restoring * src/security/security_driver.h: Add a virSecurityDomainSetSavedStateLabelRO method for labelling a saved image for restore * src/security/security_selinux.c: Implement labelling of RO save images for restore --- src/qemu/qemu_driver.c | 31 ++++++++++++++++++++----------- src/security/security_driver.h | 5 +++++ src/security/security_selinux.c | 11 +++++++++++ 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a4a87ac..d500e14 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3484,7 +3484,7 @@ static int qemudDomainSave(virDomainPtr dom, if (driver->securityDriver && driver->securityDriver->domainRestoreSavedStateLabel && - driver->securityDriver->domainRestoreSavedStateLabel(dom->conn, path) == -1) + driver->securityDriver->domainRestoreSavedStateLabel(dom->conn, vm, path) == -1) goto endjob; ret = 0; @@ -4036,6 +4036,11 @@ static int qemudDomainRestore(virConnectPtr conn, if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) goto cleanup; + if (driver->securityDriver && + driver->securityDriver->domainSetSavedStateLabelRO && + driver->securityDriver->domainSetSavedStateLabelRO(conn, vm, path) == -1) + goto endjob; + if (header.version == 2) { const char *intermediate_argv[3] = { NULL, "-dc", NULL }; const char *prog = qemudSaveCompressionTypeToString(header.compressed); @@ -4070,15 +4075,8 @@ static int qemudDomainRestore(virConnectPtr conn, close(intermediatefd); close(fd); fd = -1; - if (ret < 0) { - if (!vm->persistent) { - qemuDomainObjEndJob(vm); - virDomainRemoveInactive(&driver->domains, - vm); - vm = NULL; - } + if (ret < 0) goto endjob; - } event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, @@ -4102,8 +4100,19 @@ static int qemudDomainRestore(virConnectPtr conn, ret = 0; endjob: - if (vm) - qemuDomainObjEndJob(vm); + qemuDomainObjEndJob(vm); + if (driver->securityDriver && + driver->securityDriver->domainRestoreSavedStateLabel && + driver->securityDriver->domainRestoreSavedStateLabel(conn, vm, path) == -1) + VIR_WARN("Unable to restore labelling on %s", path); + + if (ret < 0) { + if (!vm->persistent) { + virDomainRemoveInactive(&driver->domains, + vm); + vm = NULL; + } + } cleanup: virDomainDefFree(def); diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 5514962..5144976 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -45,7 +45,11 @@ typedef int (*virSecurityDomainSetHostdevLabel) (virConnectPtr conn, typedef int (*virSecurityDomainSetSavedStateLabel) (virConnectPtr conn, virDomainObjPtr vm, const char *savefile); +typedef int (*virSecurityDomainSetSavedStateLabelRO) (virConnectPtr conn, + virDomainObjPtr vm, + const char *savefile); typedef int (*virSecurityDomainRestoreSavedStateLabel) (virConnectPtr conn, + virDomainObjPtr vm, const char *savefile); typedef int (*virSecurityDomainGenLabel) (virConnectPtr conn, virDomainObjPtr sec); @@ -77,6 +81,7 @@ struct _virSecurityDriver { virSecurityDomainRestoreHostdevLabel domainRestoreSecurityHostdevLabel; virSecurityDomainSetHostdevLabel domainSetSecurityHostdevLabel; virSecurityDomainSetSavedStateLabel domainSetSavedStateLabel; + virSecurityDomainSetSavedStateLabelRO domainSetSavedStateLabelRO; virSecurityDomainRestoreSavedStateLabel domainRestoreSavedStateLabel; /* diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index bd838e6..49f6746 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -637,7 +637,17 @@ SELinuxSetSavedStateLabel(virConnectPtr conn, static int +SELinuxSetSavedStateLabelRO(virConnectPtr conn, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + const char *savefile) +{ + return SELinuxSetFilecon(conn, savefile, default_content_context); +} + + +static int SELinuxRestoreSavedStateLabel(virConnectPtr conn, + virDomainObjPtr vm ATTRIBUTE_UNUSED, const char *savefile) { return SELinuxRestoreSecurityFileLabel(conn, savefile); @@ -714,5 +724,6 @@ virSecurityDriver virSELinuxSecurityDriver = { .domainSetSecurityHostdevLabel = SELinuxSetSecurityHostdevLabel, .domainRestoreSecurityHostdevLabel = SELinuxRestoreSecurityHostdevLabel, .domainSetSavedStateLabel = SELinuxSetSavedStateLabel, + .domainSetSavedStateLabelRO = SELinuxSetSavedStateLabelRO, .domainRestoreSavedStateLabel = SELinuxRestoreSavedStateLabel, }; -- 1.6.5.2

2 1

[libvirt] race between qemu monitor startup and blocking migrate -incoming
by Charles Duffy 19 Nov '09

19 Nov '09

Howdy, 'yall. I'm having issues with virDomainRestore failing, particularly under load -- even in 0.7.0, when there's no need to parse through qemu's output to find the monitor PTY. Digging through strace output of libvirtd and the qemu processes it spawns, this is happening when qemu blocks on the migrate -incoming and ceases to respond to the monitor socket -- though some versions of qemu can go into this state before the monitor socket is even opened, leading to libvirt timing out either while attempting to open the monitor socket or while trying to read therefrom, and subsequently killing the qemu instance it spawned while that instance is still attempting to migrate in its old saved state. Both of qemu-0.11.0-rc1 and qemu-kvm master have some form of blocking in -incoming exec: which can prevent libvirt from successfully carrying through a resume; I have reproduced the issue (and maintain logs from strace, available on request) irrespective of the state of Chris Lalancette's "Fix detached migration with exec" and "Allow monitor interaction when using migrate -exec" patches. The qemu binaries being used _appear_ to correctly allow monitor interaction prior to -incoming exec:... completion when interactively invoked in the trivial case shown below: $ qemu-system-x86_64 \ -monitor stdio \ -nographic \ -serial file:/dev/null \ -incoming 'exec:sleep 5; echo DONE >&2; kill $PPID' \ /dev/null QEMU 0.10.91 monitor - type 'help' for more information (qemu) DONE $ ...however, whether these same binaries work as-expected when invoked from libvirt by our automated test system under load is nondeterministic. (I have yet to reproduce the issue in a low-load environment using "virsh restore"). Is someone else working on this? Is a known-good (or believed-good) libvirt/qemu pair available? What can I do to help in getting this issue resolved? Thanks! --- libvirt-0.7.0 + qemu-kvm-0.11.0-rc1 qemudReadMonitorOutput:728 : internal error Timed out while reading monitor startup output libvirt-0.6.5 + qemu-kvm-0.11.0-rc1 error : qemudReadMonitorOutput:705 : internal error Timed out while reading monitor startup output error : qemudWaitForMonitor:1003 : internal error unable to start guest: char device redirected to /dev/pts/9 libvir: QEMU error : internal error unable to start guest: char device redirected to /dev/pts/9 ^^ particularly interesting, as the above line should have been eaten by qemudExtractMonitorPath rather than emitted as error text --- <aliguori> -incoming is blocking <aliguori> you cannot interact with the monitor during -incoming <mDuff> ...shouldn't we always be opening the monitor before starting the blocking -incoming bits, though? I don't always see that happening (and have an strace handy where it certainly doesn't). <aliguori> no <aliguori> well, i think they added some patches for that <aliguori> but originally, that's not how it worked <aliguori> and i think it's silly to work that way <aliguori> -incoming should mean, wait patiently for an incoming migration <aliguori> there's no point in interfacing with the monitor in the interim <mDuff> I agree that interacting may not be called for, but at least connect()ing -- if it's a UNIX socket, the other side won't be able to connect at all until qemu goes first... <aliguori> heh, well.... <aliguori> that particular race condition is addressed by -daemonize <aliguori> because that's generally true <aliguori> you don't know how long qemu will take to open the monitor <aliguori> but -daemonize makes gives you notification because it doesn't daemonize the process until you've gotten to the point where all sockets are open <aliguori> but IIRC, libvirt doesn't use -daemonize

3 2

[libvirt] iSCSI Multi-IQN (Libvirt Support)
by Shyam_Iyer＠Dell.com 19 Nov '09

19 Nov '09

The following patch set realizes the multi-IQN concept discussed in an earlier thread http://www.mail-archive.com/libvir-list@redhat.com/msg16706.html The patch realizes an XML schema like the one below and allows libvirt to read through it to create storage pools. These XMLs when created using a virtualization manager realize unique VM to storage LUN mappings through a single console and thus opening up possibilities for the following scenarios - * possibility of multiple IQNs for a single Guest * option for hypervisor's initiator to use these IQNs on behalf of the guest Example Usages: Usage 1: VM1 - > <Init iqn1> <------------------------> <Target iqn1> <Init iqn2> <------------------------> <Target iqn1> <Init iqn3> <------------------------> <Target iqn1> <Init iqn4> <------------------------> <Target iqn1> Usage 2: VM1 - > <Init iqn1> <------------------------> <Target iqn1> <Init iqn2> <------------------------> <Target iqn2> <Init iqn3> <------------------------> <Target iqn3> <Init iqn4> <------------------------> <Target iqn4> Usage 3: VM1 - > <Init iqn1> <------------------------> <Target iqn1> VM2 - > <Init iqn2> <------------------------> <Target iqn1> Usage 4: VM1 - > <Init iqn1> <------------------------> <Target iqn1> VM2 - > <Init iqn2> <------------------------> <Target iqn2> Example XML schema for an iSCSI storage pool created -- <pool type="iscsi"> <name>dell</name> <uuid>cf354733-b01b-8870-2040-94888640e24d</uuid> <capacity>0</capacity> <allocation>0</allocation> <available>0</available> - <source> <initiator> iqnname = "<initiator IQN1>"> iqnname = "<initiator IQN2>"> </initiator> ........................................ ........................................ <host name="<iSCSI target hostname or Target IP address>" /> <device path="<iSCSI Target IQN name>" /> </source> - <target> <path>/dev/disk/by-path</path> - <permissions> <mode>0700</mode> <owner>0</owner> <group>0</group> </permissions> </target> </pool> Each initiator iqn name can be parsed to create the unique sessions. TODO: 1) Virt Manager GUI support to allow a visual mapping of iqn->storage pool -> VM and thus creating the XML for consumption by libvirt. Libvirt support added above can realize the same using virsh options. 2) option for Guest's own BIOS & initiator to use the initiator IQNs (iSCSI in Guest) a) Libvirt support to allow iqn as an attribute for a VM's XML. b) Qemu Support to allow Guest's bios & initiator to use these IQNs. Signed-off-by: Sudhir Bellad <sudhir_bellad(a)dell.com> Signed-off-by: Shyam Iyer <shyam_iyer(a)dell.com> diff --git a/src/storage_backend_iscsi.c b/src/storage_backend_iscsi.c index b516add..3f2a79d 100644 --- a/src/storage_backend_iscsi.c +++ b/src/storage_backend_iscsi.c @@ -39,6 +39,10 @@ #include "storage_backend_iscsi.h" #include "util.h" #include "memory.h" +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> +#include <unistd.h> #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -159,13 +163,57 @@ virStorageBackendISCSIConnection(virConnectPtr conn, const char *portal, const char *action) { - const char *const cmdargv[] = { - ISCSIADM, "--mode", "node", "--portal", portal, - "--targetname", pool->def->source.devices[0].path, action, NULL - }; - - if (virRun(conn, cmdargv, NULL) < 0) - return -1; + DIR *dir; + struct dirent *entry; + + + if (pool->def->source.initiator[0].iqnname != NULL) { + int i = 0; + while(pool->def->source.initiator[i].iqnname != NULL){ + if (!(dir = opendir(IFACES_DIR))) { + if (errno == ENOENT) + return 0; + virReportSystemError(conn, errno, _("Failed to open dir '%s'"), + IFACES_DIR); + return -1; + } + while ((entry = readdir(dir))) { + FILE *fp; + char path[PATH_MAX]; + + if (entry->d_name[0] == '.') + continue; + + sprintf(path,"%s", IFACES_DIR); + strcat(path,(const char *)entry->d_name); + + if ((fp = fopen(path, "r"))){ + char buff[256]; + while (fp != NULL && fgets(buff, sizeof(buff), fp) != NULL) { + if (strstr(buff, pool->def->source.initiator[i].iqnname) != NULL) { + const char *const cmdargv[] = { + ISCSIADM, "--mode", "node", "--portal", portal, + "--targetname", pool->def->source.devices[0].path, "-I", entry->d_name, action, NULL + }; + + if (virRun(conn, cmdargv, NULL) < 0) + return -1; + } + } + } + fclose(fp); + } + i++; + } + } + else{ + const char *const cmdargv[] = { + ISCSIADM, "--mode", "node", "--portal", portal, + "--targetname", pool->def->source.devices[0].path, action, NULL + }; + if (virRun(conn, cmdargv, NULL) < 0) + return -1; + } return 0; } diff --git a/src/storage_backend_iscsi.h b/src/storage_backend_iscsi.h index 665ed13..14c2c5c 100644 --- a/src/storage_backend_iscsi.h +++ b/src/storage_backend_iscsi.h @@ -25,7 +25,7 @@ #define __VIR_STORAGE_BACKEND_ISCSI_H__ #include "storage_backend.h" - extern virStorageBackend virStorageBackendISCSI; +#define IFACES_DIR "/var/lib/iscsi/ifaces/" #endif /* __VIR_STORAGE_BACKEND_ISCSI_H__ */ diff --git a/src/storage_conf.c b/src/storage_conf.c index 788de15..0f2ace9 100644 --- a/src/storage_conf.c +++ b/src/storage_conf.c @@ -106,12 +106,13 @@ struct _virStorageVolOptions { /* Flags to indicate mandatory components in the pool source */ enum { - VIR_STORAGE_POOL_SOURCE_HOST = (1<<0), - VIR_STORAGE_POOL_SOURCE_DEVICE = (1<<1), - VIR_STORAGE_POOL_SOURCE_DIR = (1<<2), - VIR_STORAGE_POOL_SOURCE_ADAPTER = (1<<3), - VIR_STORAGE_POOL_SOURCE_NAME = (1<<4), -}; + VIR_STORAGE_POOL_SOURCE_HOST = (1<<0), + VIR_STORAGE_POOL_SOURCE_DEVICE = (1<<1), + VIR_STORAGE_POOL_SOURCE_DIR = (1<<2), + VIR_STORAGE_POOL_SOURCE_ADAPTER = (1<<3), + VIR_STORAGE_POOL_SOURCE_NAME = (1<<4), + VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN = (1<<5), + }; @@ -179,7 +180,8 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { { .poolType = VIR_STORAGE_POOL_ISCSI, .poolOptions = { .flags = (VIR_STORAGE_POOL_SOURCE_HOST | - VIR_STORAGE_POOL_SOURCE_DEVICE), + VIR_STORAGE_POOL_SOURCE_DEVICE | + VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN), }, .volOptions = { .formatToString = virStoragePoolFormatDiskTypeToString, @@ -532,6 +534,34 @@ virStoragePoolDefParseXML(virConnectPtr conn, goto cleanup; } } + + if(options->flags & VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN) { + xmlNodePtr *nodeset = NULL; + int niqn, i; + + if((niqn = virXPathNodeSet(conn, "./initiator/iqnname", ctxt, &nodeset)) < 0) { + virStorageReportError(conn, VIR_ERR_XML_ERROR, + "%s", _("cannot extract storage pool source devices")); + goto cleanup; + } + if (VIR_ALLOC_N(ret->source.initiator, niqn) < 0) { + VIR_FREE(nodeset); + virReportOOMError(conn); + goto cleanup; + } + for (i = 0 ; i < niqn ; i++) { + xmlChar *name = xmlGetProp(nodeset[i], BAD_CAST "name"); + if (name == NULL) { + VIR_FREE(nodeset); + virStorageReportError(conn, VIR_ERR_XML_ERROR, + "%s", _("missing storage pool source device path")); + goto cleanup; + } + ret->source.initiator[i].iqnname = (char *)name; + } + VIR_FREE(nodeset); + } + if (options->flags & VIR_STORAGE_POOL_SOURCE_DEVICE) { xmlNodePtr *nodeset = NULL; int nsource, i; diff --git a/src/storage_conf.h b/src/storage_conf.h index 9fedb12..c77d6ae 100644 --- a/src/storage_conf.h +++ b/src/storage_conf.h @@ -182,6 +182,13 @@ struct _virStoragePoolSourceDeviceExtent { int type; /* free space type */ }; +typedef struct _virStoragePoolSourceInitiatorAttr virStoragePoolSourceInitiatorAttr; +typedef virStoragePoolSourceInitiatorAttr *virStoragePoolSourceInitiatorAttrPtr; +struct _virStoragePoolSourceInitiatorAttr { + /* Initiator iqn name */ + char *iqnname; +}; + /* * Pools can be backed by one or more devices, and some * allow us to track free space on underlying devices. @@ -223,6 +230,9 @@ struct _virStoragePoolSource { /* Or a name */ char *name; + /* One or more initiator names */ + virStoragePoolSourceInitiatorAttrPtr initiator; + int authType; /* virStoragePoolAuthType */ union { virStoragePoolAuthChap chap;

7 27

[libvirt] [PATCH] Ensure driver lock is released when entering QEMU monitor
by Daniel P. Berrange 19 Nov '09

19 Nov '09

The qemudStartVMDaemon() and several functions it calls use the QEMU monitor. The QEMU driver is locked while this function is executing, so it is rquired to release the driver lock and reacquire it either side of issuing a monitor command. It failed todo so, leading to deadlock * qemu/qemu_driver.c: Release driver when in qemudStartVMDaemon and things it calls --- src/qemu/qemu_driver.c | 29 ++++++++++++++++------------- 1 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 968118e..a4a87ac 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -118,6 +118,7 @@ static void qemudShutdownVMDaemon(virConnectPtr conn, static int qemudDomainGetMaxVcpus(virDomainPtr dom); static int qemuDetectVcpuPIDs(virConnectPtr conn, + struct qemud_driver *driver, virDomainObjPtr vm); static int qemuUpdateActivePciHostdevs(struct qemud_driver *driver, @@ -1314,6 +1315,7 @@ qemudWaitForMonitor(virConnectPtr conn, static int qemuDetectVcpuPIDs(virConnectPtr conn, + struct qemud_driver *driver, virDomainObjPtr vm) { pid_t *cpupids = NULL; int ncpupids; @@ -1331,12 +1333,12 @@ qemuDetectVcpuPIDs(virConnectPtr conn, /* What follows is now all KVM specific */ - qemuDomainObjEnterMonitor(vm); + qemuDomainObjEnterMonitorWithDriver(driver, vm); if ((ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids)) < 0) { - qemuDomainObjExitMonitor(vm); + qemuDomainObjExitMonitorWithDriver(driver, vm); return -1; } - qemuDomainObjExitMonitor(vm); + qemuDomainObjExitMonitorWithDriver(driver, vm); /* Treat failure to get VCPU<->PID mapping as non-fatal */ if (ncpupids == 0) @@ -1357,6 +1359,7 @@ qemuDetectVcpuPIDs(virConnectPtr conn, static int qemudInitCpus(virConnectPtr conn, + struct qemud_driver *driver, virDomainObjPtr vm, const char *migrateFrom) { #if HAVE_SCHED_GETAFFINITY @@ -1397,15 +1400,15 @@ qemudInitCpus(virConnectPtr conn, /* XXX This resume doesn't really belong here. Move it up to caller */ if (migrateFrom == NULL) { /* Allow the CPUS to start executing */ - qemuDomainObjEnterMonitor(vm); + qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuMonitorStartCPUs(priv->mon, conn) < 0) { if (virGetLastError() == NULL) qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("resume operation failed")); - qemuDomainObjExitMonitor(vm); + qemuDomainObjExitMonitorWithDriver(driver, vm); return -1; } - qemuDomainObjExitMonitor(vm); + qemuDomainObjExitMonitorWithDriver(driver, vm); } return 0; @@ -1422,12 +1425,12 @@ qemuInitPasswords(struct qemud_driver *driver, vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && (vm->def->graphics[0]->data.vnc.passwd || driver->vncPassword)) { - qemuDomainObjEnterMonitor(vm); + qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorSetVNCPassword(priv->mon, vm->def->graphics[0]->data.vnc.passwd ? vm->def->graphics[0]->data.vnc.passwd : driver->vncPassword); - qemuDomainObjExitMonitor(vm); + qemuDomainObjExitMonitorWithDriver(driver, vm); } return ret; @@ -2294,21 +2297,21 @@ static int qemudStartVMDaemon(virConnectPtr conn, if (qemudWaitForMonitor(conn, driver, vm, pos) < 0) goto abort; - if (qemuDetectVcpuPIDs(conn, vm) < 0) + if (qemuDetectVcpuPIDs(conn, driver, vm) < 0) goto abort; - if (qemudInitCpus(conn, vm, migrateFrom) < 0) + if (qemudInitCpus(conn, driver, vm, migrateFrom) < 0) goto abort; if (qemuInitPasswords(driver, vm) < 0) goto abort; - qemuDomainObjEnterMonitor(vm); + qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuMonitorSetBalloon(priv->mon, vm->def->memory) < 0) { - qemuDomainObjExitMonitor(vm); + qemuDomainObjExitMonitorWithDriver(driver, vm); goto abort; } - qemuDomainObjExitMonitor(vm); + qemuDomainObjExitMonitorWithDriver(driver, vm); if (virDomainSaveStatus(conn, driver->stateDir, vm) < 0) goto abort; -- 1.6.5.2

2 1