- Devel - libvirt List Archives

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

19 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 18 Nov '09

18 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 18 Nov '09

18 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

Re: [libvirt] migration: qemu vs. qemu+tcp at virsh vs. libvirt-java
by Thomas Treutner 18 Nov '09

18 Nov '09

On Wednesday 18 November 2009 14:05:03 Bryan Kearney wrote: > thanks.. I am in the code right now.. what would have been better in the > javadoc for you? I will put it in. ... "If uri is NULL, then libvirt will try to find the best method. Uri may specify the hostname or IP address of the destination host as seen from the source, or uri may be a URI giving transport, hostname, user, port, etc. in the usual form. Uri should only be specified if you want to migrate over a specific interface on the remote host. For Qemu/KVM, the uri should be of the form "tcp://hostname[:port]". This does not require TCP auth to be setup between the connections, since migrate uses a straight TCP connection (unless using the PEER2PEER flag, in which case URI should be a full fledged libvirt URI). Refer also to driver documentation for the particular URIs supported." ... @Cole: I've merged your comment - could you please (N)ACK that? I assume the "tcp://.." form is a special case for Qemu/KVM, as with Xen, an IP was enough. kr tom

2 1

[libvirt] migration: qemu vs. qemu+tcp at virsh vs. libvirt-java
by Thomas Treutner 18 Nov '09

18 Nov '09

Hi, I'm having a little problem when triggering (live) migration from libvirt-java (libvirt.so at client is approx. 0.7.1), I get an error that the qemu:// driver for migration URIs is not supported, only qemu+tcp:// Strange thing is, with virsh at the same client (same libvirt.so), qemu:// works. Interestingly, the error message seems to come from libvirt.so and not libvirt-java. I'd like to avoid maintaining two different URIs/drivers for normal connections and migration destinations, especially as it works flawlessly (and fast!) with virsh? Has anyone clues where this twisted-mind behaviour could come from? kr, tom

3 4

[libvirt] RFC exposing SRIOV PF<->VF relationships
by Dave Allan 17 Nov '09

17 Nov '09

It would be helpful to have an API that exposes the relationship between SR IOV physical functions (PFs) and virtual functions (VFs). I'm thinking of implementing three new API calls and adding a small amount of additional information in the node device pci device capability, as in the attached patch. Let me know what you think. Dave >From 6ad5d2d6f06647295cde128a6c309e645e283146 Mon Sep 17 00:00:00 2001 From: David Allan <dallan(a)redhat.com> Date: Mon, 16 Nov 2009 16:06:00 -0500 Subject: [PATCH 1/1] Added basis of SR IOV PF<->VF relationship support * Added three functions to node device API * Added a little additional information to the node device PCI device capabilities to track the PF<->VFs relationships. --- src/conf/node_device_conf.h | 3 +++ src/driver.h | 11 +++++++++++ 2 files changed, 14 insertions(+), 0 deletions(-) diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 639a7e7..a130dd9 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -105,6 +105,9 @@ struct _virNodeDevCapsDef { unsigned class; char *product_name; char *vendor_name; + char *physical_function; + char **virtual_functions; + unsigned num_virtual_functions; } pci_dev; struct { unsigned bus; diff --git a/src/driver.h b/src/driver.h index 09ce8e2..c818ced 100644 --- a/src/driver.h +++ b/src/driver.h @@ -805,6 +805,14 @@ typedef int (*virDevMonDeviceListCaps)(virNodeDevicePtr dev, char **const names, int maxnames); +typedef int (*virDevMonDeviceNumOfVFs)(virNodeDevicePtr dev); + +typedef int (*virDevMonDeviceListVFs)(virNodeDevicePtr dev, + char **const names, + int maxnames); + +typedef int (*virDevMonDeviceGetPF)(virNodeDevicePtr dev); + typedef virNodeDevicePtr (*virDrvNodeDeviceCreateXML)(virConnectPtr conn, const char *xmlDesc, unsigned int flags); @@ -828,6 +836,9 @@ struct _virDeviceMonitor { virDevMonDeviceGetParent deviceGetParent; virDevMonDeviceNumOfCaps deviceNumOfCaps; virDevMonDeviceListCaps deviceListCaps; + virDevMonDeviceNumOfVFs deviceNumOfVFs; + virDevMonDeviceListVFs deviceListVFs; + virDevMonDeviceGetPF deviceGetPF; virDrvNodeDeviceCreateXML deviceCreateXML; virDrvNodeDeviceDestroy deviceDestroy; }; -- 1.6.5.1

2 2

[libvirt] PATCH 0/2]: Fix CPU affinity management for NR_CPUS > 1024
by Daniel P. Berrange 17 Nov '09

17 Nov '09

The RHEL-6 Alpha kernels have defined NR_CPUS=4096 The glibc static cpu_set_t data type used with sched_[gs]etaffinity() has been sized with # define __CPU_SETSIZE 1024 So any attempt to use sched_[gs]etaffinity() with a static cpu_set_t on a RHEL-6 kernel ends up with -EINVAL, even if the machine does not physically have > 1024 CPUs. The syscall mandates that the cpu set arg is large enough to hold entire of NR_CPUS. Fairly recent gLibc's now provide an alternative dynamically allocated cpu_set_t data type, where the app tells glibc how many CPUs it has to be able to hold. Great, except, the kernel does not tell userspace what NR_CPUS it has been compiled with. So apps have to a pick a starting size, try the sched_[gs]etaffinity() call, catch EINVAL, enlarge the cpuset, repeat, repeat, repeat until it works. This was going to be horrible to put into the qemu driver code, so I've introduced a new util/processinfo.[h,c] file providing APis for getting/setting CPU affinity that use the dynamically allocate mask libvirt defines in its public API. Daniel

2 6