April 2009 - Devel - libvirt List Archives

[libvirt] [PATCH 2/2] support for multiple graphics devices
by Pritesh Kothari 07 May '09

07 May '09

Hi All, I have added support for multiple graphics devices, the patches are as below. I have checked them against current cvs head and works fine PATCH 1/2: contains changes in libvirt for multiple graphics devices PATCH 2/2: contains corresponding changes in qemu driver. Regards, Pritesh

3 5

[libvirt] PATCH: Raise log level for dlopen() problems
by Daniel P. Berrange 06 May '09

06 May '09

Some of the problems when dlopen'ing a module really should be reported to the user/admin more readily. So this raises the logging level for the important failure messages, so they're visible by default Daniel diff -r 15c4668d403b src/driver.c --- a/src/driver.c Wed Apr 29 15:29:17 2009 +0100 +++ b/src/driver.c Thu Apr 30 14:49:27 2009 +0100 @@ -54,13 +54,13 @@ virDriverLoadModule(const char *name) return NULL; if (access(modfile, R_OK) < 0) { - DEBUG("Moodule %s not accessible", modfile); + VIR_WARN("Module %s not accessible", modfile); goto cleanup; } handle = dlopen(modfile, RTLD_NOW | RTLD_LOCAL); if (!handle) { - DEBUG("failed to load module %s %s", modfile, dlerror()); + VIR_ERROR("failed to load module %s %s", modfile, dlerror()); goto cleanup; } @@ -70,12 +70,12 @@ virDriverLoadModule(const char *name) regsym = dlsym(handle, regfunc); if (!regsym) { - DEBUG("Missing module registration symbol %s", regfunc); + VIR_ERROR("Missing module registration symbol %s", regfunc); goto cleanup; } if ((*regsym)() < 0) { - DEBUG("Failed module registration %s", regfunc); + VIR_ERROR("Failed module registration %s", regfunc); goto cleanup; } -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 1

[libvirt] PATCH: More robust name & UUID uniqueness checking for QEMU
by Daniel P. Berrange 06 May '09

06 May '09

When defining a VM config, we need to apply the following logi - If existing VM has same UUID - If name also matches => allow - Else => raise error - Else - If name matches => raise error - Else => allow When creating a live VM, or restoring a VM we need to apply similar, but slightly different logic - If existing VM has same UUID - If name also matches - If existing VM is running => raise error - Else => allow - Else => raise error - Else - If name matches => raise error - Else => allow This patch applies those checks for the QEMU driver Daniel diff -r 4e6a98395da5 src/qemu_driver.c --- a/src/qemu_driver.c Thu Apr 30 14:50:14 2009 +0100 +++ b/src/qemu_driver.c Thu Apr 30 15:03:03 2009 +0100 @@ -2145,22 +2145,37 @@ static virDomainPtr qemudDomainCreate(vi if (virSecurityDriverVerify(conn, def) < 0) goto cleanup; - vm = virDomainFindByName(&driver->domains, def->name); - if (vm) { - qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined"), - def->name); - goto cleanup; - } + /* See if a VM with matching UUID already exists */ vm = virDomainFindByUUID(&driver->domains, def->uuid); if (vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - - virUUIDFormat(def->uuid, uuidstr); - qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain with uuid '%s' is already defined"), - uuidstr); - goto cleanup; + /* UUID matches, but if names don't match, refuse it */ + if (STRNEQ(vm->def->name, def->name)) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(vm->def->uuid, uuidstr); + qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("domain '%s' is already defined with uuid %s"), + vm->def->name, uuidstr); + goto cleanup; + } + + /* UUID & name match, but if VM is already active, refuse it */ + if (virDomainIsActive(vm)) { + qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("domain is already active as '%s'"), vm->def->name); + goto cleanup; + } + virDomainObjUnlock(vm); + } else { + /* UUID does not match, but if a name matches, refuse it */ + vm = virDomainFindByName(&driver->domains, def->name); + if (vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(vm->def->uuid, uuidstr); + qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("domain '%s' is already defined with uuid %s"), + def->name, uuidstr); + goto cleanup; + } } if (!(vm = virDomainAssignDef(conn, @@ -2348,6 +2363,11 @@ static int qemudDomainDestroy(virDomainP _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } + if (!virDomainIsActive(vm)) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("domain is not running")); + goto cleanup; + } qemudShutdownVMDaemon(dom->conn, driver, vm); event = virDomainEventNewFromObj(vm, @@ -3258,17 +3278,36 @@ static int qemudDomainRestore(virConnect goto cleanup; } - /* Ensure the name and UUID don't already exist in an active VM */ + /* See if a VM with matching UUID already exists */ vm = virDomainFindByUUID(&driver->domains, def->uuid); - if (!vm) - vm = virDomainFindByName(&driver->domains, def->name); if (vm) { + /* UUID matches, but if names don't match, refuse it */ + if (STRNEQ(vm->def->name, def->name)) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(vm->def->uuid, uuidstr); + qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("domain '%s' is already defined with uuid %s"), + vm->def->name, uuidstr); + goto cleanup; + } + + /* UUID & name match, but if VM is already active, refuse it */ if (virDomainIsActive(vm)) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_INVALID, _("domain is already active as '%s'"), vm->def->name); goto cleanup; - } else { - virDomainObjUnlock(vm); + } + virDomainObjUnlock(vm); + } else { + /* UUID does not match, but if a name matches, refuse it */ + vm = virDomainFindByName(&driver->domains, def->name); + if (vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(vm->def->uuid, uuidstr); + qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("domain '%s' is already defined with uuid %s"), + def->name, uuidstr); + goto cleanup; } } @@ -3603,18 +3642,41 @@ static virDomainPtr qemudDomainDefine(vi if (virSecurityDriverVerify(conn, def) < 0) goto cleanup; - vm = virDomainFindByName(&driver->domains, def->name); + /* See if a VM with matching UUID already exists */ + vm = virDomainFindByUUID(&driver->domains, def->uuid); if (vm) { + /* UUID matches, but if names don't match, refuse it */ + if (STRNEQ(vm->def->name, def->name)) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(vm->def->uuid, uuidstr); + qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("domain '%s' is already defined with uuid %s"), + vm->def->name, uuidstr); + goto cleanup; + } + + /* UUID & name match */ virDomainObjUnlock(vm); newVM = 0; + } else { + /* UUID does not match, but if a name matches, refuse it */ + vm = virDomainFindByName(&driver->domains, def->name); + if (vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(vm->def->uuid, uuidstr); + qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("domain '%s' is already defined with uuid %s"), + def->name, uuidstr); + goto cleanup; + } } if (!(vm = virDomainAssignDef(conn, &driver->domains, def))) { - virDomainDefFree(def); - goto cleanup; - } + goto cleanup; + } + def = NULL; vm->persistent = 1; if (virDomainSaveConfig(conn, @@ -3636,6 +3698,7 @@ static virDomainPtr qemudDomainDefine(vi if (dom) dom->id = vm->def->id; cleanup: + virDomainDefFree(def); if (vm) virDomainObjUnlock(vm); if (event) -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 1

[libvirt] PATCH: Don't reset / detach host PCI devices in test scripts !
by Daniel P. Berrange 06 May '09

06 May '09

The PCI passthrough patches made it so that qemudBuildCommandLine() would actually try to detach your host devices & reset them. Most definitely not what you want when running this via a test case! This patch moves the host device management out into a separate method, so that qemudBuildCommandLine() doesn't do anything except safely build the command line. Daniel qemu_conf.c | 46 ----------------------------------- qemu_driver.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 46 deletions(-) Index: src/qemu_conf.c =================================================================== RCS file: /data/cvs/libvirt/src/qemu_conf.c,v retrieving revision 1.133 diff -u -p -u -p -r1.133 qemu_conf.c --- src/qemu_conf.c 2 Mar 2009 20:22:35 -0000 1.133 +++ src/qemu_conf.c 2 Mar 2009 20:47:36 -0000 @@ -47,7 +47,6 @@ #include "datatypes.h" #include "xml.h" #include "nodeinfo.h" -#include "pci.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -1395,52 +1394,7 @@ int qemudBuildCommandLine(virConnectPtr ADD_ARG_LIT("-pcidevice"); ADD_ARG_LIT(pcidev); VIR_FREE(pcidev); - - if (hostdev->managed) { - pciDevice *dev = pciGetDevice(conn, - hostdev->source.subsys.u.pci.domain, - hostdev->source.subsys.u.pci.bus, - hostdev->source.subsys.u.pci.slot, - hostdev->source.subsys.u.pci.function); - if (!dev) - goto error; - - if (pciDettachDevice(conn, dev) < 0) { - pciFreeDevice(conn, dev); - goto error; - } - - pciFreeDevice(conn, dev); - } /* else { - XXX validate that non-managed device isn't in use, eg - by checking that device is either un-bound, or bound - to pci-stub.ko - } */ } - - } - - /* Now that all the PCI hostdevs have be dettached, we can reset them */ - for (i = 0 ; i < vm->def->nhostdevs ; i++) { - virDomainHostdevDefPtr hostdev = vm->def->hostdevs[i]; - pciDevice *dev; - - if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || - hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) - continue; - - dev = pciGetDevice(conn, - hostdev->source.subsys.u.pci.domain, - hostdev->source.subsys.u.pci.bus, - hostdev->source.subsys.u.pci.slot, - hostdev->source.subsys.u.pci.function); - if (!dev) - goto error; - - if (pciResetDevice(conn, dev) < 0) - goto error; - - pciFreeDevice(conn, dev); } if (migrateFrom) { Index: src/qemu_driver.c =================================================================== RCS file: /data/cvs/libvirt/src/qemu_driver.c,v retrieving revision 1.208 diff -u -p -u -p -r1.208 qemu_driver.c --- src/qemu_driver.c 2 Mar 2009 17:39:43 -0000 1.208 +++ src/qemu_driver.c 2 Mar 2009 20:47:36 -0000 @@ -1133,6 +1133,79 @@ static int qemudNextFreeVNCPort(struct q return -1; } +static int qemuPrepareHostDevices(virConnectPtr conn, + virDomainDefPtr def) { + int i; + + /* We have to use 2 loops here. *All* devices must + * be detached before we reset any of them, because + * in some cases you have to reset the whole PCI bus, + * which impacts all devices on it + */ + + for (i = 0 ; i < def->nhostdevs ; i++) { + virDomainHostdevDefPtr hostdev = def->hostdevs[i]; + + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + continue; + if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) + continue; + + if (!hostdev->managed) { + pciDevice *dev = pciGetDevice(conn, + hostdev->source.subsys.u.pci.domain, + hostdev->source.subsys.u.pci.bus, + hostdev->source.subsys.u.pci.slot, + hostdev->source.subsys.u.pci.function); + if (!dev) + goto error; + + if (pciDettachDevice(conn, dev) < 0) { + pciFreeDevice(conn, dev); + goto error; + } + + pciFreeDevice(conn, dev); + } /* else { + XXX validate that non-managed device isn't in use, eg + by checking that device is either un-bound, or bound + to pci-stub.ko + } */ + } + + /* Now that all the PCI hostdevs have be dettached, we can safely + * reset them */ + for (i = 0 ; i < def->nhostdevs ; i++) { + virDomainHostdevDefPtr hostdev = def->hostdevs[i]; + pciDevice *dev; + + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + continue; + if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) + continue; + + dev = pciGetDevice(conn, + hostdev->source.subsys.u.pci.domain, + hostdev->source.subsys.u.pci.bus, + hostdev->source.subsys.u.pci.slot, + hostdev->source.subsys.u.pci.function); + if (!dev) + goto error; + + if (pciResetDevice(conn, dev) < 0) { + pciFreeDevice(conn, dev); + goto error; + } + + pciFreeDevice(conn, dev); + } + + return 0; + +error: + return -1; +} + static virDomainPtr qemudDomainLookupByName(virConnectPtr conn, const char *name); @@ -1210,6 +1283,9 @@ static int qemudStartVMDaemon(virConnect return -1; } + if (qemuPrepareHostDevices(conn, vm->def) < 0) + return -1; + vm->def->id = driver->nextvmid++; if (qemudBuildCommandLine(conn, driver, vm, qemuCmdFlags, &argv, &progenv, -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

4 5

[libvirt] [PATCH] Refresh QEMU driver caps in getCapabilities
by Cole Robinson 06 May '09

06 May '09

Hi all, The attached patch fixes QEMU getCapabilities to refresh caps before returning them to the user. Currently the capabilities are only refreshed once (at daemon startup), which means libvirtd needs to be restarted to pick up changes if QEMU or KVM are installed while the daemon is running. See: https://bugzilla.redhat.com/show_bug.cgi?id=460649 There are several things 'wrong' with this change: - We reset/rescan fields that won't change (host arch, mac address prefix). This should be fixed at some point, but isn't a big deal since total performance impact is negligible (see below). - We only refresh the capabilities when the user calls getCapabilities, which means we are still carrying around stale caps prior to that, which is what libvirt validates against. In practice, virt-manager and virt-install both call getCapabilities often so this isn't an issue, though the caps internal API should probably address this at some point. To test the performance impact, I used a simple python script: import libvirt conn = libvirt.open("qemu:///system") for i in range(0, 30): conn.getCapabilities() The time difference was on average .02 seconds slower, which I think is negligible. If at somepoint in the future, capabilities generation becomes smarter (searching PATH emulators, scraping device list output, etc.) it might be worth re-checking the time impact. But for now it doesn't seem to be an issue. Thanks, Cole

4 7

[libvirt] [PATCH 1/1] Changes in version detection and moving a global variable in the global struct
by Pritesh Kothari 06 May '09

06 May '09

Hi All, PATCH 1/1: contains changes in version detection and moving a global variable in the global struct, so that no more global vairables exists. Regards, Pritesh

3 4

[libvirt] [PATCH 1/1] Making path detection more robust in VirtualBox
by Pritesh Kothari 06 May '09

06 May '09

Hi All, PATCH 1/1: contains changes in the glue code for making path detection more robust in VirtualBox Regards, Pritesh

3 2

[libvirt] [PATCH 0/1] Added Networking API to change/create/etc. hostonly/internal network in VirtualBox
by Pritesh Kothari 06 May '09

06 May '09

Hi All: [PATCH 0/1]: Contains sample xml file showing features supported so far. [PATCH 1/1]: Contains the patch for adding Networking API to change/create/etc. hostonly/internal network in VirtualBox Regards, Pritesh

2 2

[libvirt] [PATCH 1/2] Support for 'hostonly' and 'internal' network in VirtualBox
by Pritesh Kothari 06 May '09

06 May '09

Hi All, I have added the support for hostonly and internal network as suggested on irc and list and am attaching the patch for same. PATCH 1/2: contains changes in the xml parsing/formatting on libvirt side namely adding a mode attribute to <source> tag in domain/interface xml PATCH 2/2: contains changes in the vbox driver itself. Regards, Pritesh

2 1

[libvirt] [PATCH 1/1] Added Networking API to change/create/etc. hostonly/internal network in VirtualBox
by Pritesh Kothari 06 May '09

06 May '09

Hi All: [PATCH 0/1]: Contains sample xml file showing features supported so far. [PATCH 1/1]: Contains the patch for adding Networking API to change/create/etc. hostonly/internal network in VirtualBox Regards, Pritesh

2 1