[libvirt] PATCH: More robust name & UUID uniqueness checking for QEMU
by Daniel P. Berrange
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 :|
15 years, 6 months
[libvirt] PATCH: Don't reset / detach host PCI devices in test scripts !
by Daniel P. Berrange
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 :|
15 years, 6 months
[libvirt] [PATCH] Refresh QEMU driver caps in getCapabilities
by Cole Robinson
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
15 years, 6 months
[libvirt] [PATCH 1/2] Support for 'hostonly' and 'internal' network in VirtualBox
by Pritesh Kothari
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
15 years, 6 months
[libvirt] [PATCH 0/7] Improve QEMU process startup error reporting
by Cole Robinson
Error reporting for failed QEMU processes isn't too great at the moment.
A couple cases I have hit:
- Trying to start a VM with a cdrom pointing to a nonexistent file. The
svirt hook function choked on this and indicated as much in the domain
log, but I still had to wait for the pidfile timeout (10 seconds), and
the only error reported was 'Domain did not show up'.
- QEMU dies quickly at startup. Happened with the pulse audio issue, or
if we build cli args incorrectly. The reporting here varies: if QEMU dies
before writing out the pidfile, the above case applies. If the pidfile is
written, we timeout in ReadLogOutput, and it's hit or miss whether we even
report the logfile output to the user.
The following patches improve the above cases by ensuring we never have to
wait for a timeout, and that the logfile output is always reported in the
error message.
Cole Robinson (7):
Add documentation for __virExec and virExec
virExec: Delay daemonizing as long as possible.
Add helper function virExecDaemonize
Report qemu log data if we fail to daemonize the process.
Add pidfile argument to __virExec
Check that QEMU is still alive while reading startup output.
Don't throw away QEMU startup errors when migrating.
src/libvirt_private.syms | 2 +-
src/proxy_internal.c | 16 +--
src/qemu_conf.c | 7 -
src/qemu_driver.c | 105 +++++++----
src/remote_internal.c | 15 +--
src/uml_driver.c | 11 +-
src/util.c | 186 +++++++++++++++-----
src/util.h | 15 ++-
.../qemuxml2argvdata/qemuxml2argv-boot-cdrom.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-boot-floppy.args | 2 +-
.../qemuxml2argv-boot-network.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-bootloader.args | 2 +-
.../qemuxml2argv-clock-localtime.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-clock-utc.args | 2 +-
.../qemuxml2argv-console-compat.args | 2 +-
.../qemuxml2argv-disk-cdrom-empty.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-disk-cdrom.args | 2 +-
.../qemuxml2argv-disk-drive-boot-cdrom.args | 2 +-
.../qemuxml2argv-disk-drive-boot-disk.args | 2 +-
.../qemuxml2argv-disk-drive-cache-v1-none.args | 2 +-
.../qemuxml2argv-disk-drive-cache-v1-wb.args | 2 +-
.../qemuxml2argv-disk-drive-cache-v1-wt.args | 2 +-
.../qemuxml2argv-disk-drive-cache-v2-none.args | 2 +-
.../qemuxml2argv-disk-drive-cache-v2-wb.args | 2 +-
.../qemuxml2argv-disk-drive-cache-v2-wt.args | 2 +-
.../qemuxml2argv-disk-drive-fmt-qcow.args | 2 +-
.../qemuxml2argv-disk-drive-shared.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-disk-floppy.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-disk-many.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-disk-usb.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-disk-virtio.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-disk-xenvbd.args | 2 +-
.../qemuxml2argv-graphics-sdl-fullscreen.args | 2 +-
.../qemuxml2argv-graphics-sdl.args | 2 +-
.../qemuxml2argv-graphics-vnc-sasl.args | 2 +-
.../qemuxml2argv-graphics-vnc-tls.args | 2 +-
.../qemuxml2argv-graphics-vnc.args | 2 +-
.../qemuxml2argv-hostdev-pci-address.args | 2 +-
.../qemuxml2argv-hostdev-usb-address.args | 2 +-
.../qemuxml2argv-hostdev-usb-product.args | 2 +-
.../qemuxml2argv-input-usbmouse.args | 2 +-
.../qemuxml2argv-input-usbtablet.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-input-xen.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-migrate.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-minimal.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-misc-acpi.args | 2 +-
.../qemuxml2argv-misc-no-reboot.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-misc-uuid.args | 2 +-
.../qemuxml2argv-net-eth-ifname.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-net-eth.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-net-user.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-net-virtio.args | 2 +-
.../qemuxml2argv-parallel-tcp.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-restore-v1.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-restore-v2.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-serial-dev.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-serial-file.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-serial-many.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-serial-pty.args | 2 +-
.../qemuxml2argv-serial-tcp-telnet.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-serial-tcp.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-serial-udp.args | 2 +-
.../qemuxml2argvdata/qemuxml2argv-serial-unix.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-serial-vc.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-sound.args | 2 +-
65 files changed, 288 insertions(+), 183 deletions(-)
15 years, 6 months
