- Devel - libvirt List Archives

[libvirt] PATCH: Change virDomainAssignDef to lookup on UUID
by Daniel P. Berrange 20 Apr '09

20 Apr '09

The virDomainAssignDef method currently uses the 'name' field to lookup domains. The UUID is the best internal identifier, so this changes over to use that instead. Daniel diff -r 4b9560b57861 src/domain_conf.c --- a/src/domain_conf.c Fri Apr 17 11:50:54 2009 +0100 +++ b/src/domain_conf.c Fri Apr 17 12:05:11 2009 +0100 @@ -498,7 +498,7 @@ virDomainObjPtr virDomainAssignDef(virCo { virDomainObjPtr domain; - if ((domain = virDomainFindByName(doms, def->name))) { + if ((domain = virDomainFindByUUID(doms, def->uuid))) { if (!virDomainIsActive(domain)) { virDomainDefFree(domain->def); domain->def = def; -- |: 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 2

[libvirt] PATCH: Fix remote driver create/destroy methods to update ID field
by Daniel P. Berrange 20 Apr '09

20 Apr '09

If you have an existing virDomainPtr object, and start it using the virDomainCreate(virDomainPtr dom) method, then internal cached 'id' field in the virDomainPtr object is never updated. Even more annoyingly the remote protocol for the 'create' method doesn't even bother to return the ID of the newly started guest. Thre is a similar problem with the virDomainDestroy method, not resetting the cached 'id' back to -1. We can't guarentee that the 'id' field is up2date wrt to changes made by another libvirt client, but we can at least make sure its accurate wrt to changes this client is making. For the destroy method the fix is trivial. For the create method, after a successful creation, we do a lookup based on UUID to fetch the real live ID, since the create method didn't return it for us. This fixes a significant number of problems identified by the TCK on the QEMU driver usage. Regards, Daniel diff -r 9ff786a7908d src/remote_internal.c --- a/src/remote_internal.c Fri Apr 17 12:05:11 2009 +0100 +++ b/src/remote_internal.c Fri Apr 17 12:06:21 2009 +0100 @@ -1939,6 +1939,7 @@ remoteDomainDestroy (virDomainPtr domain goto done; rv = 0; + domain->id = -1; done: remoteDriverUnlock(priv); @@ -2735,6 +2736,8 @@ remoteDomainCreate (virDomainPtr domain) { int rv = -1; remote_domain_create_args args; + remote_domain_lookup_by_uuid_args args2; + remote_domain_lookup_by_uuid_ret ret2; struct private_data *priv = domain->conn->privateData; remoteDriverLock(priv); @@ -2746,6 +2749,16 @@ remoteDomainCreate (virDomainPtr domain) (xdrproc_t) xdr_void, (char *) NULL) == -1) goto done; + memcpy (args2.uuid, domain->uuid, VIR_UUID_BUFLEN); + memset (&ret2, 0, sizeof ret2); + if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_LOOKUP_BY_UUID, + (xdrproc_t) xdr_remote_domain_lookup_by_uuid_args, (char *) &args2, + (xdrproc_t) xdr_remote_domain_lookup_by_uuid_ret, (char *) &ret2) == -1) + goto done; + + domain->id = ret2.dom.id; + xdr_free ((xdrproc_t) &xdr_remote_domain_lookup_by_uuid_ret, (char *) &ret2); + rv = 0; done: -- |: 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 2

[libvirt] PATCH: Improve error reporting for operations on inactive domains
by Daniel P. Berrange 20 Apr '09

20 Apr '09

If you try todo an operation on an inactive QEMU guest which is not applicable, eg ask to pause an inactive guest, then you currently get a useless message # virsh suspend demo error: Failed to suspend domain demo error: invalid domain pointer in no domain with matching id -1 There are two issues here - The QEMU driver is mistakenly doing a lookup-by-id when locating the guest in question. It should in fact do lookup-by-uuid for all APIs since that's the best unique identifier. - It was using VIR_ERR_INVALID_DOMAIN code instead of VIR_ERR_NO_DOMAIN code, hence the 'invalid domain pointer' bogus message. This patch changes all QEMU APIs to lookup based on UUID, and use the correct VIR_ERR_NO_DOMAIN code when reporting failures. You now get to see the real useful error message later in the API where it validates whether the guest is running or not # virsh suspend demo error: Failed to suspend domain demo error: operation failed: domain is not running One thing I'm wondering, is whether we should introduce an explicit error code for operations that are not applicable. eg, instead of giving VIR_ERR_OPERATION_FAILED, we could give back a code like VIR_ERR_OPERATION_INVALID. This would let callers distinguish real failure of the operation, vs the fact that it simply isn't applicable for inactive guests. Daniel diff -r 5b88ef324d90 src/qemu_driver.c --- a/src/qemu_driver.c Fri Apr 17 12:06:21 2009 +0100 +++ b/src/qemu_driver.c Fri Apr 17 12:32:54 2009 +0100 @@ -1984,7 +1984,8 @@ static virDomainPtr qemudDomainLookupByI qemuDriverUnlock(driver); if (!vm) { - qemudReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL); + qemudReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching id %d"), id); goto cleanup; } @@ -2008,7 +2009,10 @@ static virDomainPtr qemudDomainLookupByU qemuDriverUnlock(driver); if (!vm) { - qemudReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(uuid, uuidstr); + qemudReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuid); goto cleanup; } @@ -2032,7 +2036,8 @@ static virDomainPtr qemudDomainLookupByN qemuDriverUnlock(driver); if (!vm) { - qemudReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL); + qemudReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching name '%s'"), name); goto cleanup; } @@ -2182,11 +2187,14 @@ static int qemudDomainSuspend(virDomainP virDomainEventPtr event = NULL; qemuDriverLock(driver); - vm = virDomainFindByID(&driver->domains, dom->id); - qemuDriverUnlock(driver); - - if (!vm) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, _("no domain with matching id %d"), dom->id); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } if (!virDomainIsActive(vm)) { @@ -2232,12 +2240,14 @@ static int qemudDomainResume(virDomainPt virDomainEventPtr event = NULL; qemuDriverLock(driver); - vm = virDomainFindByID(&driver->domains, dom->id); - qemuDriverUnlock(driver); - - if (!vm) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, - _("no domain with matching id %d"), dom->id); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } if (!virDomainIsActive(vm)) { @@ -2281,12 +2291,14 @@ static int qemudDomainShutdown(virDomain int ret = -1; qemuDriverLock(driver); - vm = virDomainFindByID(&driver->domains, dom->id); - qemuDriverUnlock(driver); - - if (!vm) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, - _("no domain with matching id %d"), dom->id); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -2312,10 +2324,12 @@ static int qemudDomainDestroy(virDomainP virDomainEventPtr event = NULL; qemuDriverLock(driver); - vm = virDomainFindByID(&driver->domains, dom->id); - if (!vm) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, - _("no domain with matching id %d"), dom->id); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -2349,8 +2363,10 @@ static char *qemudDomainGetOSType(virDom vm = virDomainFindByUUID(&driver->domains, dom->uuid); qemuDriverUnlock(driver); if (!vm) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, - "%s", _("no domain with matching uuid")); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -2375,9 +2391,8 @@ static unsigned long qemudDomainGetMaxMe if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; - - virUUIDFormat(dom->uuid, uuidstr); - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -2401,9 +2416,8 @@ static int qemudDomainSetMaxMemory(virDo if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; - - virUUIDFormat(dom->uuid, uuidstr); - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -2525,9 +2539,8 @@ static int qemudDomainSetMemory(virDomai qemuDriverUnlock(driver); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; - - virUUIDFormat(dom->uuid, uuidstr); - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -2566,8 +2579,10 @@ static int qemudDomainGetInfo(virDomainP vm = virDomainFindByUUID(&driver->domains, dom->uuid); qemuDriverUnlock(driver); if (!vm) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, - "%s", _("no domain with matching uuid")); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -2713,11 +2728,13 @@ static int qemudDomainSave(virDomainPtr header.version = QEMUD_SAVE_VERSION; qemuDriverLock(driver); - vm = virDomainFindByID(&driver->domains, dom->id); - - if (!vm) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, - _("no domain with matching id %d"), dom->id); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -2850,9 +2867,8 @@ static int qemudDomainSetVcpus(virDomain if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; - - virUUIDFormat(dom->uuid, uuidstr); - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -3045,9 +3061,8 @@ static int qemudDomainGetMaxVcpus(virDom if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; - - virUUIDFormat(dom->uuid, uuidstr); - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -3080,9 +3095,8 @@ static int qemudDomainGetSecurityLabel(v if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; - - virUUIDFormat(dom->uuid, uuidstr); - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -3294,8 +3308,10 @@ static char *qemudDomainDumpXML(virDomai qemuDriverUnlock(driver); if (!vm) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, - "%s", _("no domain with matching uuid")); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -3496,8 +3512,10 @@ static int qemudDomainStart(virDomainPtr qemuDriverUnlock(driver); if (!vm) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, - "%s", _("no domain with matching uuid")); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -3586,8 +3604,10 @@ static int qemudDomainUndefine(virDomain vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, - "%s", _("no domain with matching uuid")); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -3979,9 +3999,11 @@ static int qemudDomainAttachDevice(virDo qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { - qemuDriverUnlock(driver); - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, - "%s", _("no domain with matching uuid")); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + qemuDriverUnlock(driver); + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -4129,9 +4151,11 @@ static int qemudDomainDetachDevice(virDo qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { - qemuDriverUnlock(driver); - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, - "%s", _("no domain with matching uuid")); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + qemuDriverUnlock(driver); + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -4182,8 +4206,10 @@ static int qemudDomainGetAutostart(virDo qemuDriverUnlock(driver); if (!vm) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, - "%s", _("no domain with matching uuid")); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -4208,8 +4234,10 @@ static int qemudDomainSetAutostart(virDo qemuDriverUnlock(driver); if (!vm) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, - "%s", _("no domain with matching uuid")); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -4283,11 +4311,13 @@ qemudDomainBlockStats (virDomainPtr dom, virDomainDiskDefPtr disk = NULL; qemuDriverLock(driver); - vm = virDomainFindByID(&driver->domains, dom->id); - qemuDriverUnlock(driver); - if (!vm) { - qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, - _("no domain with matching id %d"), dom->id); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } if (!virDomainIsActive (vm)) { @@ -4418,12 +4448,14 @@ qemudDomainInterfaceStats (virDomainPtr int ret = -1; qemuDriverLock(driver); - vm = virDomainFindByID(&driver->domains, dom->id); - qemuDriverUnlock(driver); - - if (!vm) { - qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, - _("no domain with matching id %d"), dom->id); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -4486,8 +4518,10 @@ qemudDomainBlockPeek (virDomainPtr dom, qemuDriverUnlock(driver); if (!vm) { - qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, - "%s", _("no domain with matching uuid")); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -4554,12 +4588,14 @@ qemudDomainMemoryPeek (virDomainPtr dom, int fd = -1, ret = -1; qemuDriverLock(driver); - vm = virDomainFindByID(&driver->domains, dom->id); - qemuDriverUnlock(driver); - - if (!vm) { - qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, - _("no domain with matching id %d"), dom->id); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -4887,10 +4923,12 @@ qemudDomainMigratePerform (virDomainPtr int paused = 0; qemuDriverLock(driver); - vm = virDomainFindByID(&driver->domains, dom->id); - if (!vm) { - qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, - _("no domain with matching id %d"), dom->id); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -5018,8 +5056,8 @@ qemudDomainMigrateFinish2 (virConnectPtr qemuDriverLock(driver); vm = virDomainFindByName(&driver->domains, dname); if (!vm) { - qemudReportError (dconn, NULL, NULL, VIR_ERR_INVALID_DOMAIN, - _("no domain with matching name %s"), dname); + qemudReportError (dconn, NULL, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching name '%s'"), dname); 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 2

[libvirt] [PATCH 1/1] lxc: only do CLONE_NEWUSER when kernel supports it
by Serge E. Hallyn 20 Apr '09

20 Apr '09

I was trying to get the lxc driver to work on ubuntu jaunty. This patch gets me further than I was getting before. Like I say below, it's probably not the right way though. -serge >From 2513f8a7e0654e84570fe0ef2204dabe276b9e4e Mon Sep 17 00:00:00 2001 From: root <root(a)jaunty.(none)> Date: Fri, 17 Apr 2009 16:41:01 -0500 Subject: [PATCH 1/1] lxc: only do CLONE_NEWUSER when kernel supports it The ubuntu jaunty kernel is not compiled with USER_NS. Since libvirt-lxc always does clone(CLONE_NEWUSER) it gets -EINVAL and mysteriously claims to be unable to contact hypervisor. This patch isn't the right thing to do, but I'm not sure what is. User namespaces do (since recently) isolate the in-kernel keyring. So the right thing might be to add a flag to the xml definition file to specify whether to use a user namespace. This patch doesn't do that, rather it always does CLONE_NEWUSER if the kernel supports it, and never if not. Signed-off-by: Serge Hallyn <serue(a)us.ibm.com> --- src/lxc_container.c | 15 +++++++++++++-- src/lxc_container.h | 1 + 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/lxc_container.c b/src/lxc_container.c index 67c66bd..8069af7 100644 --- a/src/lxc_container.c +++ b/src/lxc_container.c @@ -666,6 +666,11 @@ static int lxcContainerChild( void *data ) return lxcContainerExecInit(vmDef); } +int userns_supported(void) +{ + return lxcContainerAvailable(LXC_CONTAINER_FEATURE_USER) == 0; +} + /** * lxcContainerStart: * @driver: pointer to driver structure @@ -694,7 +699,10 @@ int lxcContainerStart(virDomainDefPtr def, } stacktop = stack + stacksize; - flags = CLONE_NEWPID|CLONE_NEWNS|CLONE_NEWUTS|CLONE_NEWUSER|CLONE_NEWIPC|SIGCHLD; + flags = CLONE_NEWPID|CLONE_NEWNS|CLONE_NEWUTS|CLONE_NEWIPC|SIGCHLD; + + if (userns_supported()) + flags |= CLONE_NEWUSER; if (def->nets != NULL) flags |= CLONE_NEWNET; @@ -719,13 +727,16 @@ static int lxcContainerDummyChild(void *argv ATTRIBUTE_UNUSED) int lxcContainerAvailable(int features) { - int flags = CLONE_NEWPID|CLONE_NEWNS|CLONE_NEWUTS|CLONE_NEWUSER| + int flags = CLONE_NEWPID|CLONE_NEWNS|CLONE_NEWUTS| CLONE_NEWIPC|SIGCHLD; int cpid; char *childStack; char *stack; int childStatus; + if (features & LXC_CONTAINER_FEATURE_USER) + flags |= CLONE_NEWUSER; + if (features & LXC_CONTAINER_FEATURE_NET) flags |= CLONE_NEWNET; diff --git a/src/lxc_container.h b/src/lxc_container.h index 5d037b0..b99e83e 100644 --- a/src/lxc_container.h +++ b/src/lxc_container.h @@ -28,6 +28,7 @@ enum { LXC_CONTAINER_FEATURE_NET = (1 << 0), + LXC_CONTAINER_FEATURE_USER = (1 << 1), }; #define LXC_DEV_MAJ_MEMORY 1 -- 1.6.0.4

3 2

[libvirt] [PATCH] Fix typo in manpage
by Mark McLoughlin 20 Apr '09

20 Apr '09

Hey, Just committed this simple patch. Cheers, Mark. Subject: [PATCH] Fix typo in manpage It's "service libvirtd start", not "service start libvirtd". Reported-by: Robert P. J. Day <rpjday(a)crashcourse.ca> Signed-off-by: Mark McLoughlin <markmc(a)redhat.com> --- docs/virsh.pod | 2 +- virsh.1 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/virsh.pod b/docs/virsh.pod index 2438984..6434d78 100644 --- a/docs/virsh.pod +++ b/docs/virsh.pod @@ -34,7 +34,7 @@ the program. All B<virsh> operations rely upon the libvirt library. For any virsh commands to run xend/qemu, or what ever virtual library that libvirt supports. For this reason you should start xend/qemu as a service when your system first boots using xen/qemu. This can usually be done using the command -B<service start libvirtd> . +B<service libvirtd start> . Most B<virsh> commands require root privileges to run due to the communications channels used to talk to the hypervisor. Running as diff --git a/virsh.1 b/virsh.1 index 2197c20..45ea614 100644 --- a/virsh.1 +++ b/virsh.1 @@ -171,7 +171,7 @@ the program. .IX Header "NOTES" All \fBvirsh\fR operations rely upon the libvirt library. For any virsh commands to run xend/qemu, or what ever virtual library that libvirt supports. For this reason you should start xend/qemu as a service when your system first boots using xen/qemu. This can usually be done using the command -\&\fBservice start libvirtd\fR . +\&\fBservice libvirtd start\fR . .PP Most \fBvirsh\fR commands require root privileges to run due to the communications channels used to talk to the hypervisor. Running as -- 1.6.0.6

1 0

[libvirt] [PATCH] src/bridge.c compile error
by Anton Protopopov 20 Apr '09

20 Apr '09

Hi. The absense of SIOCBRADDBR causes compile error: > bridge.c:143: error: conflicting types for 'brAddBridge' > bridge.h:50: error: previous declaration of 'brAddBridge' was here See attached patch for fix. By the way, what functionality do one loose if his system isn't supporting SIOCBRADDBR?

2 1

[libvirt] KVM migration with uri = NULL
by Zvi Dubitzky 20 Apr '09

20 Apr '09

What is the meaning of using virDomainMigrate () with NULL uri. Looking at the code of qemudDomainMigratePrepare2 ()/ qemu_driver.c, I understand that it tries to perform a migration to another VM on the same node . Is that correct ? Anyhow the code gets stuck while doing this with KVM-84 driver which does ok a live migration between 2 different nodes via QEMU monitor thanks Zvi Dubitzky Virtualization and System Architecture Email:dubi@il.ibm.com IBM Haifa Research Laboratory Phone: +972-4-8296182 Haifa, 31905, ISRAEL

3 2

[libvirt] [PATCH 0/3] Drop pool lock during volume allocation
by Cole Robinson 17 Apr '09

17 Apr '09

The following three patches rearrange storage volume creation so that we can drop the pool lock during the long allocation process. This also has the nice side effect of allowing storage allocation progress reporting, if the API user polls the volume in a separate thread. This is currently only implemented for FS volumes: all other volume creation will maintain existing behavior. Thanks, Cole

3 12

[libvirt] KVM live migration with libvirt
by Zvi Dubitzky 17 Apr '09

17 Apr '09

while trying a live migration for KVM with libvirt I call virDomainMigrate () with the following parameters virDomainMigrate (localDomain, (virConnectPtr)dconn, VIR_MIGRATE_LIVE, "vm_mig_dubi", "tcp:9.148.4.95:4444", 0 ); The connection with the target VM was established ok Still I get the following error: libvir: QEMU error : invalid argument in only tcp URIs are supported for KVM migrations looking at the code of qemu_driver.c /qemudDomainMigratePrepare2() is see the following code (for version2 case): if (!STREQLEN (uri_in, "tcp:", 6)) { qemudReportError (dconn, NULL, NULL, VIR_ERR_INVALID_ARG, "%s", _("only tcp URIs are supported for KVM migrations")); goto cleanup; } why is the strncmp using length 6 and not 4 . I think this is the reason for the failure . If I put a NULL af ter the "tcp:" then I do not fail here (strncmp stop at the first NULL) but I fail one step later pasrsing the port because the NULL terminates the string Do we need a fix here ? thanks Zvi Dubitzky Virtualization and System Architecture Email:dubi@il.ibm.com IBM Haifa Research Laboratory Phone: +972-4-8296182 Haifa, 31905, ISRAEL

2 1

[libvirt] [PATCH 1/4] VirtualBox support
by Pritesh Kothari 17 Apr '09

17 Apr '09

Hi All, I have attached a patch which when applied on the HEAD as of today would allow virtualbox support in libvirt. It takes cares of all the stuff mentioned on the list earlier. Still if I have missed anything, please do tell me. The patches are organized as below: Patch 0/4: contains sample xml file Patch 1/4: contains diff of files already in libvirt. Patch 2/4: contains new files needed for VirtualBox support. Patch 3/4: contains support for host only and internal network. Patch 4/4: contains support for rdp in libvirt as mentioned by danpb (also had sent it separately earlier) Regards, Pritesh

2 1