The current security driver usage requires horrible code like if (driver->securityDriver && driver->securityDriver->domainSetSecurityHostdevLabel && driver->securityDriver->domainSetSecurityHostdevLabel(driver->securityDriver, vm, hostdev) < 0) This pair of checks for NULL clutters up the code, making the driver calls 2 lines longer than they really need to be. The goal of the patchset is to change the calling convention to simply if (virSecurityManagerSetHostdevLabel(driver->securityDriver, vm, hostdev) < 0) The first check for 'driver->securityDriver' being NULL is removed by introducing a 'no op' security driver that will always be present if no real driver is enabled. This guarentees driver->securityDriver != NULL. The second check for 'driver->securityDriver->domainSetSecurityHostdevLabel' being non-NULL is hidden in a new abstraction called virSecurityManager. This separates the driver callbacks, from main internal API. The addition of a virSecurityManager object, that is separate from the virSecurityDriver struct also allows for security drivers to carry state / configuration information directly. Thus the DAC/Stack drivers from src/qemu which used to pull config from 'struct qemud_driver' can now be moved into the 'src/security' directory and store their config directly. * src/qemu/qemu_conf.h, src/qemu/qemu_driver.c: Update to use new virSecurityManager APIs * src/qemu/qemu_security_dac.c, src/qemu/qemu_security_dac.h src/qemu/qemu_security_stacked.c, src/qemu/qemu_security_stacked.h: Move into src/security directory * src/security/security_stack.c, src/security/security_stack.h, src/security/security_dac.c, src/security/security_dac.h: Generic versions of previous QEMU specific drivers * src/security/security_apparmor.c, src/security/security_apparmor.h, src/security/security_driver.c, src/security/security_driver.h, src/security/security_selinux.c, src/security/security_selinux.h: Update to take virSecurityManagerPtr object as the first param in all callbacks * src/security/security_nop.c, src/security/security_nop.h: Stub implementation of all security driver APIs. * src/security/security_manager.h, src/security/security_manager.c: New internal API for invoking security drivers --- src/Makefile.am | 12 +- src/libvirt_private.syms | 33 ++- src/qemu/qemu_conf.h | 6 +- src/qemu/qemu_driver.c | 250 +++++--------- src/qemu/qemu_security_dac.c | 590 ------------------------------- src/qemu/qemu_security_dac.h | 22 -- src/qemu/qemu_security_stacked.c | 418 ---------------------- src/qemu/qemu_security_stacked.h | 22 -- src/security/security_apparmor.c | 152 ++++++--- src/security/security_apparmor.h | 2 + src/security/security_dac.c | 712 ++++++++++++++++++++++++++++++++++++++ src/security/security_dac.h | 27 ++ src/security/security_driver.c | 116 ++----- src/security/security_driver.h | 95 +++--- src/security/security_manager.c | 291 ++++++++++++++++ src/security/security_manager.h | 74 ++++ src/security/security_nop.c | 168 +++++++++ src/security/security_nop.h | 17 + src/security/security_selinux.c | 145 +++++--- src/security/security_selinux.h | 2 +- src/security/security_stack.c | 383 ++++++++++++++++++++ src/security/security_stack.h | 24 ++ 22 files changed, 2079 insertions(+), 1482 deletions(-) delete mode 100644 src/qemu/qemu_security_dac.c delete mode 100644 src/qemu/qemu_security_dac.h delete mode 100644 src/qemu/qemu_security_stacked.c delete mode 100644 src/qemu/qemu_security_stacked.h create mode 100644 src/security/security_dac.c create mode 100644 src/security/security_dac.h create mode 100644 src/security/security_manager.c create mode 100644 src/security/security_manager.h create mode 100644 src/security/security_nop.c create mode 100644 src/security/security_nop.h create mode 100644 src/security/security_stack.c create mode 100644 src/security/security_stack.h diff --git a/src/Makefile.am b/src/Makefile.am index a9a1986..66af9cf 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -273,11 +273,7 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_monitor_json.h \ qemu/qemu_driver.c qemu/qemu_driver.h \ qemu/qemu_bridge_filter.c \ - qemu/qemu_bridge_filter.h \ - qemu/qemu_security_stacked.h \ - qemu/qemu_security_stacked.c \ - qemu/qemu_security_dac.h \ - qemu/qemu_security_dac.c + qemu/qemu_bridge_filter.h XENAPI_DRIVER_SOURCES = \ xenapi/xenapi_driver.c xenapi/xenapi_driver.h \ @@ -376,7 +372,11 @@ NWFILTER_DRIVER_SOURCES = \ # Security framework and drivers for various models SECURITY_DRIVER_SOURCES = \ - security/security_driver.h security/security_driver.c + security/security_driver.h security/security_driver.c \ + security/security_nop.h security/security_nop.c \ + security/security_stack.h security/security_stack.c \ + security/security_dac.h security/security_dac.c \ + security/security_manager.h security/security_manager.c SECURITY_DRIVER_SELINUX_SOURCES = \ security/security_selinux.h security/security_selinux.c diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8bf1028..3af0210 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -646,13 +646,32 @@ virSecretDefParseFile; virSecretDefParseString; -# security.h -virSecurityDriverGetDOI; -virSecurityDriverGetModel; -virSecurityDriverInit; -virSecurityDriverSetDOI; -virSecurityDriverStartup; -virSecurityDriverVerify; +# security_driver.h +virSecurityDriverLookup; + + +# security_manager.h +virSecurityManagerClearSocketLabel; +virSecurityManagerGenLabel; +virSecurityManagerGetDOI; +virSecurityManagerGetModel; +virSecurityManagerGetProcessLabel; +virSecurityManagerNew; +virSecurityManagerNewStack; +virSecurityManagerNewDAC; +virSecurityManagerReleaseLabel; +virSecurityManagerReserveLabel; +virSecurityManagerRestoreImageLabel; +virSecurityManagerRestoreAllLabel; +virSecurityManagerRestoreHostdevLabel; +virSecurityManagerRestoreSavedStateLabel; +virSecurityManagerSetAllLabel; +virSecurityManagerSetImageLabel; +virSecurityManagerSetHostdevLabel; +virSecurityManagerSetProcessLabel; +virSecurityManagerSetSavedStateLabel; +virSecurityManagerSetSocketLabel; +virSecurityManagerVerify; # storage_conf.h diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 790ce98..ba3c7b1 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -34,7 +34,7 @@ # include "domain_conf.h" # include "domain_event.h" # include "threads.h" -# include "security/security_driver.h" +# include "security/security_manager.h" # include "cgroup.h" # include "pci.h" # include "cpu_conf.h" @@ -166,9 +166,7 @@ struct qemud_driver { int domainEventDispatching; char *securityDriverName; - virSecurityDriverPtr securityDriver; - virSecurityDriverPtr securityPrimaryDriver; - virSecurityDriverPtr securitySecondaryDriver; + virSecurityManagerPtr securityManager; char *saveImageFormat; char *dumpImageFormat; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ed1ea6b..2b901b8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -70,8 +70,6 @@ #include "pci.h" #include "hostusb.h" #include "processinfo.h" -#include "qemu_security_stacked.h" -#include "qemu_security_dac.h" #include "cgroup.h" #include "libvirt_internal.h" #include "xml.h" @@ -1395,10 +1393,7 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm) qemuDomainObjPrivatePtr priv = vm->privateData; int ret = -1; - if (driver->securityDriver && - driver->securityDriver->domainSetSecuritySocketLabel && - driver->securityDriver->domainSetSecuritySocketLabel - (driver->securityDriver,vm) < 0) { + if (virSecurityManagerSetSocketLabel(driver->securityManager, vm) < 0) { VIR_ERROR(_("Failed to set security context for monitor for %s"), vm->def->name); goto error; @@ -1416,10 +1411,7 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm) if (priv->mon == NULL) virDomainObjUnref(vm); - if (driver->securityDriver && - driver->securityDriver->domainClearSecuritySocketLabel && - driver->securityDriver->domainClearSecuritySocketLabel - (driver->securityDriver,vm) < 0) { + if (virSecurityManagerClearSocketLabel(driver->securityManager, vm) < 0) { VIR_ERROR(_("Failed to clear security context for monitor for %s"), vm->def->name); goto error; @@ -1488,10 +1480,7 @@ qemuReconnectDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaq goto error; } - if (driver->securityDriver && - driver->securityDriver->domainReserveSecurityLabel && - driver->securityDriver->domainReserveSecurityLabel(driver->securityDriver, - obj) < 0) + if (virSecurityManagerReserveLabel(driver->securityManager, obj) < 0) goto error; if (qemudVMFiltersInstantiate(conn, obj->def)) @@ -1531,30 +1520,24 @@ qemuReconnectDomains(virConnectPtr conn, struct qemud_driver *driver) static int qemudSecurityInit(struct qemud_driver *qemud_drv) { - int ret; - virSecurityDriverPtr security_drv; - - qemuSecurityStackedSetDriver(qemud_drv); - qemuSecurityDACSetDriver(qemud_drv); - - ret = virSecurityDriverStartup(&security_drv, - qemud_drv->securityDriverName, - qemud_drv->allowDiskFormatProbing); - if (ret == -1) { - VIR_ERROR0(_("Failed to start security driver")); + virSecurityManagerPtr mgr = virSecurityManagerNew(qemud_drv->securityDriverName, + qemud_drv->allowDiskFormatProbing); + if (!mgr) return -1; - } - /* No primary security driver wanted to be enabled: just setup - * the DAC driver on its own */ - if (ret == -2) { - qemud_drv->securityDriver = &qemuDACSecurityDriver; - VIR_INFO0(_("No security driver available")); + if (qemud_drv->privileged) { + virSecurityManagerPtr dac = virSecurityManagerNewDAC(qemud_drv->user, + qemud_drv->group, + qemud_drv->allowDiskFormatProbing, + qemud_drv->dynamicOwnership); + if (!dac) + return -1; + + if (!(qemud_drv->securityManager = virSecurityManagerNewStack(mgr, + dac))) + return -1; } else { - qemud_drv->securityPrimaryDriver = security_drv; - qemud_drv->securitySecondaryDriver = &qemuDACSecurityDriver; - qemud_drv->securityDriver = &qemuStackedSecurityDriver; - VIR_INFO("Initialized security driver %s", security_drv->name); + qemud_drv->securityManager = mgr; } return 0; @@ -1600,20 +1583,22 @@ qemuCreateCapabilities(virCapsPtr oldcaps, caps->ns.href = qemuDomainDefNamespaceHref; /* Security driver data */ - if (driver->securityPrimaryDriver) { - const char *doi, *model; + const char *doi, *model; - doi = virSecurityDriverGetDOI(driver->securityPrimaryDriver); - model = virSecurityDriverGetModel(driver->securityPrimaryDriver); + doi = virSecurityManagerGetDOI(driver->securityManager); + model = virSecurityManagerGetModel(driver->securityManager); + if (STREQ(model, "none")) { + model = ""; + doi = ""; + } - if (!(caps->host.secModel.model = strdup(model))) - goto no_memory; - if (!(caps->host.secModel.doi = strdup(doi))) - goto no_memory; + if (!(caps->host.secModel.model = strdup(model))) + goto no_memory; + if (!(caps->host.secModel.doi = strdup(doi))) + goto no_memory; - VIR_DEBUG("Initialized caps for security driver \"%s\" with " - "DOI \"%s\"", model, doi); - } + VIR_DEBUG("Initialized caps for security driver \"%s\" with " + "DOI \"%s\"", model, doi); return caps; @@ -3681,9 +3666,7 @@ static int qemudSecurityHook(void *data) { if (qemudInitCpuAffinity(h->vm) < 0) return -1; - if (h->driver->securityDriver && - h->driver->securityDriver->domainSetSecurityProcessLabel && - h->driver->securityDriver->domainSetSecurityProcessLabel(h->driver->securityDriver, h->vm) < 0) + if (virSecurityManagerSetProcessLabel(h->driver->securityManager, h->vm) < 0) return -1; return 0; @@ -3912,22 +3895,17 @@ static int qemudStartVMDaemon(virConnectPtr conn, /* If you are using a SecurityDriver with dynamic labelling, then generate a security label for isolation */ DEBUG0("Generating domain security label (if required)"); - if (driver->securityDriver && - driver->securityDriver->domainGenSecurityLabel) { - ret = driver->securityDriver->domainGenSecurityLabel(driver->securityDriver, - vm); - qemuDomainSecurityLabelAudit(vm, ret >= 0); - if (ret < 0) - goto cleanup; + if (virSecurityManagerGenLabel(driver->securityManager, + vm) < 0) { + qemuDomainSecurityLabelAudit(vm, false); + goto cleanup; } + qemuDomainSecurityLabelAudit(vm, true); DEBUG0("Generating setting domain security labels (if required)"); - if (driver->securityDriver && - driver->securityDriver->domainSetSecurityAllLabel && - driver->securityDriver->domainSetSecurityAllLabel(driver->securityDriver, - vm, stdin_path) < 0) { + if (virSecurityManagerSetAllLabel(driver->securityManager, + vm, stdin_path) < 0) goto cleanup; - } /* Ensure no historical cgroup for this VM is lying around bogus * settings */ @@ -4342,14 +4320,10 @@ static void qemudShutdownVMDaemon(struct qemud_driver *driver, } /* Reset Security Labels */ - if (driver->securityDriver && - driver->securityDriver->domainRestoreSecurityAllLabel) - driver->securityDriver->domainRestoreSecurityAllLabel(driver->securityDriver, - vm, migrated); - if (driver->securityDriver && - driver->securityDriver->domainReleaseSecurityLabel) - driver->securityDriver->domainReleaseSecurityLabel(driver->securityDriver, - vm); + virSecurityManagerRestoreAllLabel(driver->securityManager, + vm, migrated); + virSecurityManagerReleaseLabel(driver->securityManager, + vm); /* Clear out dynamically assigned labels */ if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) { @@ -4830,7 +4804,7 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, VIR_DOMAIN_XML_INACTIVE))) goto cleanup; - if (virSecurityDriverVerify(def) < 0) + if (virSecurityManagerVerify(driver->securityManager, def) < 0) goto cleanup; if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) @@ -5743,10 +5717,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, } if ((!bypassSecurityDriver) && - driver->securityDriver && - driver->securityDriver->domainSetSavedStateLabel && - driver->securityDriver->domainSetSavedStateLabel(driver->securityDriver, - vm, path) == -1) + virSecurityManagerSetSavedStateLabel(driver->securityManager, + vm, path) < 0) goto endjob; if (header.compressed == QEMUD_SAVE_FORMAT_RAW) { @@ -5779,10 +5751,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, goto endjob; if ((!bypassSecurityDriver) && - driver->securityDriver && - driver->securityDriver->domainRestoreSavedStateLabel && - driver->securityDriver->domainRestoreSavedStateLabel(driver->securityDriver, - vm, path) == -1) + virSecurityManagerRestoreSavedStateLabel(driver->securityManager, + vm, path) < 0) VIR_WARN("failed to restore save state label on %s", path); if (cgroup != NULL) { @@ -5828,10 +5798,8 @@ endjob: } if ((!bypassSecurityDriver) && - driver->securityDriver && - driver->securityDriver->domainRestoreSavedStateLabel && - driver->securityDriver->domainRestoreSavedStateLabel(driver->securityDriver, - vm, path) == -1) + virSecurityManagerRestoreSavedStateLabel(driver->securityManager, + vm, path) < 0) VIR_WARN("failed to restore save state label on %s", path); } @@ -6096,10 +6064,8 @@ static int qemudDomainCoreDump(virDomainPtr dom, goto endjob; } - if (driver->securityDriver && - driver->securityDriver->domainSetSavedStateLabel && - driver->securityDriver->domainSetSavedStateLabel(driver->securityDriver, - vm, path) == -1) + if (virSecurityManagerSetSavedStateLabel(driver->securityManager, + vm, path) < 0) goto endjob; /* Migrate will always stop the VM, so the resume condition is @@ -6156,11 +6122,9 @@ static int qemudDomainCoreDump(virDomainPtr dom, paused = 1; - if (driver->securityDriver && - driver->securityDriver->domainRestoreSavedStateLabel && - driver->securityDriver->domainRestoreSavedStateLabel(driver->securityDriver, - vm, path) == -1) - goto endjob; + if (virSecurityManagerRestoreSavedStateLabel(driver->securityManager, + vm, path) < 0) + VIR_WARN("Failed to restore security label on %s", path); endjob: if ((ret == 0) && (flags & VIR_DUMP_CRASH)) { @@ -6643,10 +6607,8 @@ static int qemudDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr sec * QEMU monitor hasn't seen SIGHUP/ERR on poll(). */ if (virDomainObjIsActive(vm)) { - if (driver->securityDriver && - driver->securityDriver->domainGetSecurityProcessLabel && - driver->securityDriver->domainGetSecurityProcessLabel(driver->securityDriver, - vm, seclabel) < 0) { + if (virSecurityManagerGetProcessLabel(driver->securityManager, + vm, seclabel) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to get security label")); goto cleanup; @@ -6670,11 +6632,6 @@ static int qemudNodeGetSecurityModel(virConnectPtr conn, int ret = 0; qemuDriverLock(driver); - if (!driver->securityPrimaryDriver) { - memset(secmodel, 0, sizeof (*secmodel)); - goto cleanup; - } - p = driver->caps->host.secModel.model; if (strlen(p) >= VIR_SECURITY_MODEL_BUFLEN-1) { qemuReportError(VIR_ERR_INTERNAL_ERROR, @@ -7067,10 +7024,8 @@ qemudDomainSaveImageStartVM(virConnectPtr conn, ret = 0; out: - if (driver->securityDriver && - driver->securityDriver->domainRestoreSavedStateLabel && - driver->securityDriver->domainRestoreSavedStateLabel(driver->securityDriver, - vm, path) == -1) + if (virSecurityManagerRestoreSavedStateLabel(driver->securityManager, + vm, path) < 0) VIR_WARN("failed to restore save state label on %s", path); return ret; @@ -7646,7 +7601,7 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) { VIR_DOMAIN_XML_INACTIVE))) goto cleanup; - if (virSecurityDriverVerify(def) < 0) + if (virSecurityManagerVerify(driver->securityManager, def) < 0) goto cleanup; if ((dupVM = virDomainObjIsDuplicate(&driver->domains, def, 0)) < 0) @@ -7786,10 +7741,8 @@ static int qemudDomainChangeEjectableMedia(struct qemud_driver *driver, return -1; } - if (driver->securityDriver && - driver->securityDriver->domainSetSecurityImageLabel && - driver->securityDriver->domainSetSecurityImageLabel(driver->securityDriver, - vm, disk) < 0) + if (virSecurityManagerSetImageLabel(driver->securityManager, + vm, disk) < 0) return -1; if (!(driveAlias = qemuDeviceDriveHostAlias(origdisk, qemuCmdFlags))) @@ -7818,10 +7771,8 @@ static int qemudDomainChangeEjectableMedia(struct qemud_driver *driver, if (ret < 0) goto error; - if (driver->securityDriver && - driver->securityDriver->domainRestoreSecurityImageLabel && - driver->securityDriver->domainRestoreSecurityImageLabel(driver->securityDriver, - vm, origdisk) < 0) + if (virSecurityManagerRestoreImageLabel(driver->securityManager, + vm, origdisk) < 0) VIR_WARN("Unable to restore security label on ejected image %s", origdisk->src); VIR_FREE(origdisk->src); @@ -7837,11 +7788,10 @@ static int qemudDomainChangeEjectableMedia(struct qemud_driver *driver, error: VIR_FREE(driveAlias); - if (driver->securityDriver && - driver->securityDriver->domainRestoreSecurityImageLabel && - driver->securityDriver->domainRestoreSecurityImageLabel(driver->securityDriver, - vm, disk) < 0) + if (virSecurityManagerRestoreImageLabel(driver->securityManager, + vm, disk) < 0) VIR_WARN("Unable to restore security label on new media %s", disk->src); + return -1; } @@ -7865,10 +7815,8 @@ static int qemudDomainAttachPciDiskDevice(struct qemud_driver *driver, } } - if (driver->securityDriver && - driver->securityDriver->domainSetSecurityImageLabel && - driver->securityDriver->domainSetSecurityImageLabel(driver->securityDriver, - vm, disk) < 0) + if (virSecurityManagerSetImageLabel(driver->securityManager, + vm, disk) < 0) return -1; if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { @@ -7935,10 +7883,8 @@ error: qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &disk->info) < 0) VIR_WARN("Unable to release PCI address on %s", disk->src); - if (driver->securityDriver && - driver->securityDriver->domainRestoreSecurityImageLabel && - driver->securityDriver->domainRestoreSecurityImageLabel(driver->securityDriver, - vm, disk) < 0) + if (virSecurityManagerRestoreImageLabel(driver->securityManager, + vm, disk) < 0) VIR_WARN("Unable to restore security label on %s", disk->src); return -1; @@ -8078,10 +8024,8 @@ static int qemudDomainAttachSCSIDisk(struct qemud_driver *driver, } - if (driver->securityDriver && - driver->securityDriver->domainSetSecurityImageLabel && - driver->securityDriver->domainSetSecurityImageLabel(driver->securityDriver, - vm, disk) < 0) + if (virSecurityManagerSetImageLabel(driver->securityManager, + vm, disk) < 0) return -1; /* We should have an address already, so make sure */ @@ -8167,10 +8111,8 @@ error: VIR_FREE(devstr); VIR_FREE(drivestr); - if (driver->securityDriver && - driver->securityDriver->domainRestoreSecurityImageLabel && - driver->securityDriver->domainRestoreSecurityImageLabel(driver->securityDriver, - vm, disk) < 0) + if (virSecurityManagerRestoreImageLabel(driver->securityManager, + vm, disk) < 0) VIR_WARN("Unable to restore security label on %s", disk->src); return -1; @@ -8195,10 +8137,8 @@ static int qemudDomainAttachUsbMassstorageDevice(struct qemud_driver *driver, } } - if (driver->securityDriver && - driver->securityDriver->domainSetSecurityImageLabel && - driver->securityDriver->domainSetSecurityImageLabel(driver->securityDriver, - vm, disk) < 0) + if (virSecurityManagerSetImageLabel(driver->securityManager, + vm, disk) < 0) return -1; if (!disk->src) { @@ -8254,10 +8194,8 @@ error: VIR_FREE(devstr); VIR_FREE(drivestr); - if (driver->securityDriver && - driver->securityDriver->domainRestoreSecurityImageLabel && - driver->securityDriver->domainRestoreSecurityImageLabel(driver->securityDriver, - vm, disk) < 0) + if (virSecurityManagerRestoreImageLabel(driver->securityManager, + vm, disk) < 0) VIR_WARN("Unable to restore security label on %s", disk->src); return -1; @@ -8681,10 +8619,8 @@ static int qemudDomainAttachHostDevice(struct qemud_driver *driver, } - if (driver->securityDriver && - driver->securityDriver->domainSetSecurityHostdevLabel && - driver->securityDriver->domainSetSecurityHostdevLabel(driver->securityDriver, - vm, hostdev) < 0) + if (virSecurityManagerSetHostdevLabel(driver->securityManager, + vm, hostdev) < 0) return -1; switch (hostdev->source.subsys.type) { @@ -8710,10 +8646,8 @@ static int qemudDomainAttachHostDevice(struct qemud_driver *driver, return 0; error: - if (driver->securityDriver && - driver->securityDriver->domainRestoreSecurityHostdevLabel && - driver->securityDriver->domainRestoreSecurityHostdevLabel(driver->securityDriver, - vm, hostdev) < 0) + if (virSecurityManagerRestoreHostdevLabel(driver->securityManager, + vm, hostdev) < 0) VIR_WARN0("Unable to restore host device labelling on hotplug fail"); return -1; @@ -9150,10 +9084,8 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, virDomainDiskDefFree(detach); - if (driver->securityDriver && - driver->securityDriver->domainRestoreSecurityImageLabel && - driver->securityDriver->domainRestoreSecurityImageLabel(driver->securityDriver, - vm, dev->data.disk) < 0) + if (virSecurityManagerRestoreImageLabel(driver->securityManager, + vm, dev->data.disk) < 0) VIR_WARN("Unable to restore security label on %s", dev->data.disk->src); if (cgroup != NULL) { @@ -9216,10 +9148,8 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, virDomainDiskDefFree(detach); - if (driver->securityDriver && - driver->securityDriver->domainRestoreSecurityImageLabel && - driver->securityDriver->domainRestoreSecurityImageLabel(driver->securityDriver, - vm, dev->data.disk) < 0) + if (virSecurityManagerRestoreImageLabel(driver->securityManager, + vm, dev->data.disk) < 0) VIR_WARN("Unable to restore security label on %s", dev->data.disk->src); if (cgroup != NULL) { @@ -9652,10 +9582,8 @@ static int qemudDomainDetachHostDevice(struct qemud_driver *driver, return -1; } - if (driver->securityDriver && - driver->securityDriver->domainRestoreSecurityHostdevLabel && - driver->securityDriver->domainRestoreSecurityHostdevLabel(driver->securityDriver, - vm, dev->data.hostdev) < 0) + if (virSecurityManagerRestoreHostdevLabel(driver->securityManager, + vm, dev->data.hostdev) < 0) VIR_WARN0("Failed to restore host device labelling"); return ret; diff --git a/src/qemu/qemu_security_dac.c b/src/qemu/qemu_security_dac.c deleted file mode 100644 index 55dc0c6..0000000 --- a/src/qemu/qemu_security_dac.c +++ /dev/null @@ -1,590 +0,0 @@ -/* - * Copyright (C) 2010 Red Hat, Inc. - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * QEMU POSIX DAC security driver - */ -#include <config.h> -#include <sys/types.h> -#include <sys/stat.h> -#include <fcntl.h> - -#include "qemu_security_dac.h" -#include "qemu_conf.h" -#include "datatypes.h" -#include "virterror_internal.h" -#include "util.h" -#include "memory.h" -#include "logging.h" -#include "pci.h" -#include "hostusb.h" -#include "storage_file.h" - -#define VIR_FROM_THIS VIR_FROM_QEMU - -static struct qemud_driver *driver; - -void qemuSecurityDACSetDriver(struct qemud_driver *newdriver) -{ - driver = newdriver; -} - - -static int -qemuSecurityDACSetOwnership(const char *path, int uid, int gid) -{ - VIR_INFO("Setting DAC user and group on '%s' to '%d:%d'", path, uid, gid); - - if (chown(path, uid, gid) < 0) { - struct stat sb; - int chown_errno = errno; - - if (stat(path, &sb) >= 0) { - if (sb.st_uid == uid && - sb.st_gid == gid) { - /* It's alright, there's nothing to change anyway. */ - return 0; - } - } - - if (chown_errno == EOPNOTSUPP) { - VIR_INFO("Setting user and group to '%d:%d' on '%s' not supported by filesystem", - uid, gid, path); - } else if (chown_errno == EPERM) { - VIR_INFO("Setting user and group to '%d:%d' on '%s' not permitted", - uid, gid, path); - } else if (chown_errno == EROFS) { - VIR_INFO("Setting user and group to '%d:%d' on '%s' not possible on readonly filesystem", - uid, gid, path); - } else { - virReportSystemError(chown_errno, - _("unable to set user and group to '%d:%d' on '%s'"), - uid, gid, path); - return -1; - } - } - return 0; -} - -static int -qemuSecurityDACRestoreSecurityFileLabel(const char *path) -{ - struct stat buf; - int rc = -1; - char *newpath = NULL; - - VIR_INFO("Restoring DAC user and group on '%s'", path); - - if (virFileResolveLink(path, &newpath) < 0) { - virReportSystemError(errno, - _("cannot resolve symlink %s"), path); - goto err; - } - - if (stat(newpath, &buf) != 0) - goto err; - - /* XXX record previous ownership */ - rc = qemuSecurityDACSetOwnership(newpath, 0, 0); - -err: - VIR_FREE(newpath); - return rc; -} - - -static int -qemuSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, - const char *path, - size_t depth ATTRIBUTE_UNUSED, - void *opaque ATTRIBUTE_UNUSED) -{ - return qemuSecurityDACSetOwnership(path, driver->user, driver->group); -} - - -static int -qemuSecurityDACSetSecurityImageLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm ATTRIBUTE_UNUSED, - virDomainDiskDefPtr disk) - -{ - if (!driver->privileged || !driver->dynamicOwnership) - return 0; - - return virDomainDiskDefForeachPath(disk, - driver->allowDiskFormatProbing, - false, - qemuSecurityDACSetSecurityFileLabel, - NULL); -} - - -static int -qemuSecurityDACRestoreSecurityImageLabelInt(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm ATTRIBUTE_UNUSED, - virDomainDiskDefPtr disk, - int migrated) -{ - if (!driver->privileged || !driver->dynamicOwnership) - return 0; - - /* Don't restore labels on readoly/shared disks, because - * other VMs may still be accessing these - * Alternatively we could iterate over all running - * domains and try to figure out if it is in use, but - * this would not work for clustered filesystems, since - * we can't see running VMs using the file on other nodes - * Safest bet is thus to skip the restore step. - */ - if (disk->readonly || disk->shared) - return 0; - - if (!disk->src) - return 0; - - /* If we have a shared FS & doing migrated, we must not - * change ownership, because that kills access on the - * destination host which is sub-optimal for the guest - * VM's I/O attempts :-) - */ - if (migrated) { - int rc = virStorageFileIsSharedFS(disk->src); - if (rc < 0) - return -1; - if (rc == 1) { - VIR_DEBUG("Skipping image label restore on %s because FS is shared", - disk->src); - return 0; - } - } - - return qemuSecurityDACRestoreSecurityFileLabel(disk->src); -} - - -static int -qemuSecurityDACRestoreSecurityImageLabel(virSecurityDriverPtr drv, - virDomainObjPtr vm, - virDomainDiskDefPtr disk) -{ - return qemuSecurityDACRestoreSecurityImageLabelInt(drv, vm, disk, 0); -} - - -static int -qemuSecurityDACSetSecurityPCILabel(pciDevice *dev ATTRIBUTE_UNUSED, - const char *file, - void *opaque ATTRIBUTE_UNUSED) -{ - return qemuSecurityDACSetOwnership(file, driver->user, driver->group); -} - - -static int -qemuSecurityDACSetSecurityUSBLabel(usbDevice *dev ATTRIBUTE_UNUSED, - const char *file, - void *opaque ATTRIBUTE_UNUSED) -{ - return qemuSecurityDACSetOwnership(file, driver->user, driver->group); -} - - -static int -qemuSecurityDACSetSecurityHostdevLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm, - virDomainHostdevDefPtr dev) - -{ - int ret = -1; - - if (!driver->privileged || !driver->dynamicOwnership) - return 0; - - if (dev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) - return 0; - - switch (dev->source.subsys.type) { - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: { - usbDevice *usb = usbGetDevice(dev->source.subsys.u.usb.bus, - dev->source.subsys.u.usb.device); - - if (!usb) - goto done; - - ret = usbDeviceFileIterate(usb, qemuSecurityDACSetSecurityUSBLabel, vm); - usbFreeDevice(usb); - break; - } - - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: { - pciDevice *pci = pciGetDevice(dev->source.subsys.u.pci.domain, - dev->source.subsys.u.pci.bus, - dev->source.subsys.u.pci.slot, - dev->source.subsys.u.pci.function); - - if (!pci) - goto done; - - ret = pciDeviceFileIterate(pci, qemuSecurityDACSetSecurityPCILabel, vm); - pciFreeDevice(pci); - - break; - } - - default: - ret = 0; - break; - } - -done: - return ret; -} - - -static int -qemuSecurityDACRestoreSecurityPCILabel(pciDevice *dev ATTRIBUTE_UNUSED, - const char *file, - void *opaque ATTRIBUTE_UNUSED) -{ - return qemuSecurityDACRestoreSecurityFileLabel(file); -} - - -static int -qemuSecurityDACRestoreSecurityUSBLabel(usbDevice *dev ATTRIBUTE_UNUSED, - const char *file, - void *opaque ATTRIBUTE_UNUSED) -{ - return qemuSecurityDACRestoreSecurityFileLabel(file); -} - - -static int -qemuSecurityDACRestoreSecurityHostdevLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm ATTRIBUTE_UNUSED, - virDomainHostdevDefPtr dev) - -{ - int ret = -1; - - if (!driver->privileged || !driver->dynamicOwnership) - return 0; - - if (dev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) - return 0; - - switch (dev->source.subsys.type) { - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: { - usbDevice *usb = usbGetDevice(dev->source.subsys.u.usb.bus, - dev->source.subsys.u.usb.device); - - if (!usb) - goto done; - - ret = usbDeviceFileIterate(usb, qemuSecurityDACRestoreSecurityUSBLabel, NULL); - usbFreeDevice(usb); - - break; - } - - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: { - pciDevice *pci = pciGetDevice(dev->source.subsys.u.pci.domain, - dev->source.subsys.u.pci.bus, - dev->source.subsys.u.pci.slot, - dev->source.subsys.u.pci.function); - - if (!pci) - goto done; - - ret = pciDeviceFileIterate(pci, qemuSecurityDACRestoreSecurityPCILabel, NULL); - pciFreeDevice(pci); - - break; - } - - default: - ret = 0; - break; - } - -done: - return ret; -} - - -static int -qemuSecurityDACSetChardevLabel(virDomainObjPtr vm, - virDomainChrDefPtr dev) - -{ - const virSecurityLabelDefPtr secdef = &vm->def->seclabel; - char *in = NULL, *out = NULL; - int ret = -1; - - if (secdef->type == VIR_DOMAIN_SECLABEL_STATIC) - return 0; - - switch (dev->type) { - case VIR_DOMAIN_CHR_TYPE_DEV: - case VIR_DOMAIN_CHR_TYPE_FILE: - ret = qemuSecurityDACSetOwnership(dev->data.file.path, driver->user, driver->group); - break; - - case VIR_DOMAIN_CHR_TYPE_PIPE: - if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) || - (virAsprintf(&out, "%s.out", dev->data.file.path) < 0)) { - virReportOOMError(); - goto done; - } - if ((qemuSecurityDACSetOwnership(in, driver->user, driver->group) < 0) || - (qemuSecurityDACSetOwnership(out, driver->user, driver->group) < 0)) - goto done; - ret = 0; - break; - - default: - ret = 0; - break; - } - -done: - VIR_FREE(in); - VIR_FREE(out); - return ret; -} - -static int -qemuSecurityDACRestoreChardevLabel(virDomainObjPtr vm, - virDomainChrDefPtr dev) - -{ - const virSecurityLabelDefPtr secdef = &vm->def->seclabel; - char *in = NULL, *out = NULL; - int ret = -1; - - if (secdef->type == VIR_DOMAIN_SECLABEL_STATIC) - return 0; - - switch (dev->type) { - case VIR_DOMAIN_CHR_TYPE_DEV: - case VIR_DOMAIN_CHR_TYPE_FILE: - ret = qemuSecurityDACRestoreSecurityFileLabel(dev->data.file.path); - break; - - case VIR_DOMAIN_CHR_TYPE_PIPE: - if ((virAsprintf(&out, "%s.out", dev->data.file.path) < 0) || - (virAsprintf(&in, "%s.in", dev->data.file.path) < 0)) { - virReportOOMError(); - goto done; - } - if ((qemuSecurityDACRestoreSecurityFileLabel(out) < 0) || - (qemuSecurityDACRestoreSecurityFileLabel(in) < 0)) - goto done; - ret = 0; - break; - - default: - ret = 0; - break; - } - -done: - VIR_FREE(in); - VIR_FREE(out); - return ret; -} - - -static int -qemuSecurityDACRestoreChardevCallback(virDomainDefPtr def ATTRIBUTE_UNUSED, - virDomainChrDefPtr dev, - void *opaque) -{ - virDomainObjPtr vm = opaque; - - return qemuSecurityDACRestoreChardevLabel(vm, dev); -} - - -static int -qemuSecurityDACRestoreSecurityAllLabel(virSecurityDriverPtr drv, - virDomainObjPtr vm, - int migrated) -{ - int i; - int rc = 0; - - if (!driver->privileged || !driver->dynamicOwnership) - return 0; - - VIR_DEBUG("Restoring security label on %s migrated=%d", - vm->def->name, migrated); - - for (i = 0 ; i < vm->def->nhostdevs ; i++) { - if (qemuSecurityDACRestoreSecurityHostdevLabel(drv, - vm, - vm->def->hostdevs[i]) < 0) - rc = -1; - } - for (i = 0 ; i < vm->def->ndisks ; i++) { - if (qemuSecurityDACRestoreSecurityImageLabelInt(drv, - vm, - vm->def->disks[i], - migrated) < 0) - rc = -1; - } - - if (virDomainChrDefForeach(vm->def, - false, - qemuSecurityDACRestoreChardevCallback, - vm) < 0) - rc = -1; - - if (vm->def->os.kernel && - qemuSecurityDACRestoreSecurityFileLabel(vm->def->os.kernel) < 0) - rc = -1; - - if (vm->def->os.initrd && - qemuSecurityDACRestoreSecurityFileLabel(vm->def->os.initrd) < 0) - rc = -1; - - return rc; -} - - -static int -qemuSecurityDACSetChardevCallback(virDomainDefPtr def ATTRIBUTE_UNUSED, - virDomainChrDefPtr dev, - void *opaque) -{ - virDomainObjPtr vm = opaque; - - return qemuSecurityDACSetChardevLabel(vm, dev); -} - - -static int -qemuSecurityDACSetSecurityAllLabel(virSecurityDriverPtr drv, - virDomainObjPtr vm, - const char *stdin_path ATTRIBUTE_UNUSED) -{ - int i; - - if (!driver->privileged || !driver->dynamicOwnership) - return 0; - - for (i = 0 ; i < vm->def->ndisks ; i++) { - /* XXX fixme - we need to recursively label the entriy tree :-( */ - if (vm->def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_DIR) - continue; - if (qemuSecurityDACSetSecurityImageLabel(drv, - vm, - vm->def->disks[i]) < 0) - return -1; - } - for (i = 0 ; i < vm->def->nhostdevs ; i++) { - if (qemuSecurityDACSetSecurityHostdevLabel(drv, - vm, - vm->def->hostdevs[i]) < 0) - return -1; - } - - if (virDomainChrDefForeach(vm->def, - true, - qemuSecurityDACSetChardevCallback, - vm) < 0) - return -1; - - if (vm->def->os.kernel && - qemuSecurityDACSetOwnership(vm->def->os.kernel, - driver->user, - driver->group) < 0) - return -1; - - if (vm->def->os.initrd && - qemuSecurityDACSetOwnership(vm->def->os.initrd, - driver->user, - driver->group) < 0) - return -1; - - return 0; -} - - -static int -qemuSecurityDACSetSavedStateLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm ATTRIBUTE_UNUSED, - const char *savefile) -{ - if (!driver->privileged) - return 0; - - return qemuSecurityDACSetOwnership(savefile, driver->user, driver->group); -} - - -static int -qemuSecurityDACRestoreSavedStateLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm ATTRIBUTE_UNUSED, - const char *savefile) -{ - if (!driver->privileged) - return 0; - - return qemuSecurityDACRestoreSecurityFileLabel(savefile); -} - - -static int -qemuSecurityDACSetProcessLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm ATTRIBUTE_UNUSED) -{ - DEBUG("Dropping privileges of VM to %d:%d", driver->user, driver->group); - - if (!driver->privileged) - return 0; - - if (driver->group) { - if (setregid(driver->group, driver->group) < 0) { - virReportSystemError(errno, - _("cannot change to '%d' group"), - driver->group); - return -1; - } - } - if (driver->user) { - if (setreuid(driver->user, driver->user) < 0) { - virReportSystemError(errno, - _("cannot change to '%d' user"), - driver->user); - return -1; - } - } - - return 0; -} - - - -virSecurityDriver qemuDACSecurityDriver = { - .name = "qemuDAC", - - .domainSetSecurityProcessLabel = qemuSecurityDACSetProcessLabel, - - .domainSetSecurityImageLabel = qemuSecurityDACSetSecurityImageLabel, - .domainRestoreSecurityImageLabel = qemuSecurityDACRestoreSecurityImageLabel, - - .domainSetSecurityAllLabel = qemuSecurityDACSetSecurityAllLabel, - .domainRestoreSecurityAllLabel = qemuSecurityDACRestoreSecurityAllLabel, - - .domainSetSecurityHostdevLabel = qemuSecurityDACSetSecurityHostdevLabel, - .domainRestoreSecurityHostdevLabel = qemuSecurityDACRestoreSecurityHostdevLabel, - - .domainSetSavedStateLabel = qemuSecurityDACSetSavedStateLabel, - .domainRestoreSavedStateLabel = qemuSecurityDACRestoreSavedStateLabel, -}; diff --git a/src/qemu/qemu_security_dac.h b/src/qemu/qemu_security_dac.h deleted file mode 100644 index a742f7a..0000000 --- a/src/qemu/qemu_security_dac.h +++ /dev/null @@ -1,22 +0,0 @@ -/* - * Copyright (C) 2010 Red Hat, Inc. - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * QEMU POSIX DAC security driver - */ - -#include "security/security_driver.h" -#include "qemu_conf.h" - -#ifndef __QEMU_SECURITY_DAC -# define __QEMU_SECURITY_DAC - -extern virSecurityDriver qemuDACSecurityDriver; - -void qemuSecurityDACSetDriver(struct qemud_driver *driver); - -#endif /* __QEMU_SECURITY_DAC */ diff --git a/src/qemu/qemu_security_stacked.c b/src/qemu/qemu_security_stacked.c deleted file mode 100644 index 432d095..0000000 --- a/src/qemu/qemu_security_stacked.c +++ /dev/null @@ -1,418 +0,0 @@ -/* - * Copyright (C) 2010 Red Hat, Inc. - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * QEMU stacked security driver - */ - -#include <config.h> -#include <sys/types.h> -#include <sys/stat.h> -#include <fcntl.h> - -#include "qemu_security_stacked.h" - -#include "qemu_conf.h" -#include "datatypes.h" -#include "virterror_internal.h" -#include "util.h" -#include "memory.h" -#include "logging.h" -#include "pci.h" -#include "hostusb.h" -#include "storage_file.h" - -#define VIR_FROM_THIS VIR_FROM_QEMU - - -static struct qemud_driver *driver; - -void qemuSecurityStackedSetDriver(struct qemud_driver *newdriver) -{ - driver = newdriver; -} - - -static int -qemuSecurityStackedVerify(virDomainDefPtr def) -{ - int rc = 0; - - if (driver->securitySecondaryDriver && - driver->securitySecondaryDriver->domainSecurityVerify && - driver->securitySecondaryDriver->domainSecurityVerify(def) < 0) - rc = -1; - - if (driver->securityPrimaryDriver && - driver->securityPrimaryDriver->domainSecurityVerify && - driver->securityPrimaryDriver->domainSecurityVerify(def) < 0) - rc = -1; - - return rc; -} - - -static int -qemuSecurityStackedGenLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm) -{ - int rc = 0; - - if (driver->securitySecondaryDriver && - driver->securitySecondaryDriver->domainGenSecurityLabel && - driver->securitySecondaryDriver->domainGenSecurityLabel(driver->securitySecondaryDriver, - vm) < 0) - rc = -1; - - if (driver->securityPrimaryDriver && - driver->securityPrimaryDriver->domainGenSecurityLabel && - driver->securityPrimaryDriver->domainGenSecurityLabel(driver->securityPrimaryDriver, - vm) < 0) - rc = -1; - - return rc; -} - - -static int -qemuSecurityStackedReleaseLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm) -{ - int rc = 0; - - if (driver->securitySecondaryDriver && - driver->securitySecondaryDriver->domainReleaseSecurityLabel && - driver->securitySecondaryDriver->domainReleaseSecurityLabel(driver->securitySecondaryDriver, - vm) < 0) - rc = -1; - - if (driver->securityPrimaryDriver && - driver->securityPrimaryDriver->domainReleaseSecurityLabel && - driver->securityPrimaryDriver->domainReleaseSecurityLabel(driver->securityPrimaryDriver, - vm) < 0) - rc = -1; - - return rc; -} - - -static int -qemuSecurityStackedReserveLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm) -{ - int rc = 0; - - if (driver->securitySecondaryDriver && - driver->securitySecondaryDriver->domainReserveSecurityLabel && - driver->securitySecondaryDriver->domainReserveSecurityLabel(driver->securitySecondaryDriver, - vm) < 0) - rc = -1; - - if (driver->securityPrimaryDriver && - driver->securityPrimaryDriver->domainReserveSecurityLabel && - driver->securityPrimaryDriver->domainReserveSecurityLabel(driver->securityPrimaryDriver, - vm) < 0) - rc = -1; - - return rc; -} - - -static int -qemuSecurityStackedSetSecurityImageLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm, - virDomainDiskDefPtr disk) -{ - int rc = 0; - - if (driver->securitySecondaryDriver && - driver->securitySecondaryDriver->domainSetSecurityImageLabel && - driver->securitySecondaryDriver->domainSetSecurityImageLabel(driver->securitySecondaryDriver, - vm, disk) < 0) - rc = -1; - - if (driver->securityPrimaryDriver && - driver->securityPrimaryDriver->domainSetSecurityImageLabel && - driver->securityPrimaryDriver->domainSetSecurityImageLabel(driver->securityPrimaryDriver, - vm, disk) < 0) - rc = -1; - - return rc; -} - - -static int -qemuSecurityStackedRestoreSecurityImageLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm, - virDomainDiskDefPtr disk) -{ - int rc = 0; - - if (driver->securitySecondaryDriver && - driver->securitySecondaryDriver->domainRestoreSecurityImageLabel && - driver->securitySecondaryDriver->domainRestoreSecurityImageLabel(driver->securitySecondaryDriver, - vm, disk) < 0) - rc = -1; - - if (driver->securityPrimaryDriver && - driver->securityPrimaryDriver->domainRestoreSecurityImageLabel && - driver->securityPrimaryDriver->domainRestoreSecurityImageLabel(driver->securityPrimaryDriver, - vm, disk) < 0) - rc = -1; - - return rc; -} - - -static int -qemuSecurityStackedSetSecurityHostdevLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm, - virDomainHostdevDefPtr dev) - -{ - int rc = 0; - - if (driver->securitySecondaryDriver && - driver->securitySecondaryDriver->domainSetSecurityHostdevLabel && - driver->securitySecondaryDriver->domainSetSecurityHostdevLabel(driver->securitySecondaryDriver, - vm, dev) < 0) - rc = -1; - - if (driver->securityPrimaryDriver && - driver->securityPrimaryDriver->domainSetSecurityHostdevLabel && - driver->securityPrimaryDriver->domainSetSecurityHostdevLabel(driver->securityPrimaryDriver, - vm, dev) < 0) - rc = -1; - - return rc; -} - - -static int -qemuSecurityStackedRestoreSecurityHostdevLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm, - virDomainHostdevDefPtr dev) -{ - int rc = 0; - - if (driver->securitySecondaryDriver && - driver->securitySecondaryDriver->domainRestoreSecurityHostdevLabel && - driver->securitySecondaryDriver->domainRestoreSecurityHostdevLabel(driver->securitySecondaryDriver, - vm, dev) < 0) - rc = -1; - - if (driver->securityPrimaryDriver && - driver->securityPrimaryDriver->domainRestoreSecurityHostdevLabel && - driver->securityPrimaryDriver->domainRestoreSecurityHostdevLabel(driver->securityPrimaryDriver, - vm, dev) < 0) - rc = -1; - - return rc; -} - - -static int -qemuSecurityStackedSetSecurityAllLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm, - const char *stdin_path) -{ - int rc = 0; - - if (driver->securitySecondaryDriver && - driver->securitySecondaryDriver->domainSetSecurityAllLabel && - driver->securitySecondaryDriver->domainSetSecurityAllLabel(driver->securitySecondaryDriver, - vm, stdin_path) < 0) - rc = -1; - - if (driver->securityPrimaryDriver && - driver->securityPrimaryDriver->domainSetSecurityAllLabel && - driver->securityPrimaryDriver->domainSetSecurityAllLabel(driver->securityPrimaryDriver, - vm, stdin_path) < 0) - rc = -1; - - return rc; -} - - -static int -qemuSecurityStackedRestoreSecurityAllLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm, - int migrated) -{ - int rc = 0; - - if (driver->securitySecondaryDriver && - driver->securitySecondaryDriver->domainRestoreSecurityAllLabel && - driver->securitySecondaryDriver->domainRestoreSecurityAllLabel(driver->securitySecondaryDriver, - vm, migrated) < 0) - rc = -1; - - if (driver->securityPrimaryDriver && - driver->securityPrimaryDriver->domainRestoreSecurityAllLabel && - driver->securityPrimaryDriver->domainRestoreSecurityAllLabel(driver->securityPrimaryDriver, - vm, migrated) < 0) - rc = -1; - - return rc; -} - - -static int -qemuSecurityStackedSetSavedStateLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm, - const char *savefile) -{ - int rc = 0; - - if (driver->securitySecondaryDriver && - driver->securitySecondaryDriver->domainSetSavedStateLabel && - driver->securitySecondaryDriver->domainSetSavedStateLabel(driver->securitySecondaryDriver, - vm, savefile) < 0) - rc = -1; - - if (driver->securityPrimaryDriver && - driver->securityPrimaryDriver->domainSetSavedStateLabel && - driver->securityPrimaryDriver->domainSetSavedStateLabel(driver->securityPrimaryDriver, - vm, savefile) < 0) - rc = -1; - - return rc; -} - - -static int -qemuSecurityStackedRestoreSavedStateLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm, - const char *savefile) -{ - int rc = 0; - - if (driver->securitySecondaryDriver && - driver->securitySecondaryDriver->domainRestoreSavedStateLabel && - driver->securitySecondaryDriver->domainRestoreSavedStateLabel(driver->securitySecondaryDriver, - vm, savefile) < 0) - rc = -1; - - if (driver->securityPrimaryDriver && - driver->securityPrimaryDriver->domainRestoreSavedStateLabel && - driver->securityPrimaryDriver->domainRestoreSavedStateLabel(driver->securityPrimaryDriver, - vm, savefile) < 0) - rc = -1; - - return rc; -} - - -static int -qemuSecurityStackedSetProcessLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm) -{ - int rc = 0; - - if (driver->securitySecondaryDriver && - driver->securitySecondaryDriver->domainSetSecurityProcessLabel && - driver->securitySecondaryDriver->domainSetSecurityProcessLabel(driver->securitySecondaryDriver, - vm) < 0) - rc = -1; - - if (driver->securityPrimaryDriver && - driver->securityPrimaryDriver->domainSetSecurityProcessLabel && - driver->securityPrimaryDriver->domainSetSecurityProcessLabel(driver->securityPrimaryDriver, - vm) < 0) - rc = -1; - - return rc; -} - -static int -qemuSecurityStackedGetProcessLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm, - virSecurityLabelPtr seclabel) -{ - int rc = 0; - - if (driver->securityPrimaryDriver && - driver->securityPrimaryDriver->domainGetSecurityProcessLabel && - driver->securityPrimaryDriver->domainGetSecurityProcessLabel(driver->securityPrimaryDriver, - vm, - seclabel) < 0) - rc = -1; - - return rc; -} - - -static int -qemuSecurityStackedSetSocketLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm) -{ - int rc = 0; - - if (driver->securityPrimaryDriver && - driver->securityPrimaryDriver->domainSetSecuritySocketLabel && - driver->securityPrimaryDriver->domainSetSecuritySocketLabel(driver->securityPrimaryDriver, - vm) < 0) - rc = -1; - - if (driver->securitySecondaryDriver && - driver->securitySecondaryDriver->domainSetSecuritySocketLabel && - driver->securitySecondaryDriver->domainSetSecuritySocketLabel(driver->securitySecondaryDriver, - vm) < 0) - rc = -1; - - return rc; -} - - -static int -qemuSecurityStackedClearSocketLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, - virDomainObjPtr vm) -{ - int rc = 0; - - if (driver->securitySecondaryDriver && - driver->securitySecondaryDriver->domainClearSecuritySocketLabel && - driver->securitySecondaryDriver->domainClearSecuritySocketLabel(driver->securitySecondaryDriver, - vm) < 0) - rc = -1; - - if (driver->securityPrimaryDriver && - driver->securityPrimaryDriver->domainClearSecuritySocketLabel && - driver->securityPrimaryDriver->domainClearSecuritySocketLabel(driver->securityPrimaryDriver, - vm) < 0) - rc = -1; - - return rc; -} - - -virSecurityDriver qemuStackedSecurityDriver = { - .name = "qemuStacked", - .domainSecurityVerify = qemuSecurityStackedVerify, - - .domainGenSecurityLabel = qemuSecurityStackedGenLabel, - .domainReleaseSecurityLabel = qemuSecurityStackedReleaseLabel, - .domainReserveSecurityLabel = qemuSecurityStackedReserveLabel, - - .domainGetSecurityProcessLabel = qemuSecurityStackedGetProcessLabel, - .domainSetSecurityProcessLabel = qemuSecurityStackedSetProcessLabel, - - .domainSetSecurityImageLabel = qemuSecurityStackedSetSecurityImageLabel, - .domainRestoreSecurityImageLabel = qemuSecurityStackedRestoreSecurityImageLabel, - - .domainSetSecurityAllLabel = qemuSecurityStackedSetSecurityAllLabel, - .domainRestoreSecurityAllLabel = qemuSecurityStackedRestoreSecurityAllLabel, - - .domainSetSecurityHostdevLabel = qemuSecurityStackedSetSecurityHostdevLabel, - .domainRestoreSecurityHostdevLabel = qemuSecurityStackedRestoreSecurityHostdevLabel, - - .domainSetSavedStateLabel = qemuSecurityStackedSetSavedStateLabel, - .domainRestoreSavedStateLabel = qemuSecurityStackedRestoreSavedStateLabel, - - .domainClearSecuritySocketLabel = qemuSecurityStackedClearSocketLabel, - .domainSetSecuritySocketLabel = qemuSecurityStackedSetSocketLabel, -}; diff --git a/src/qemu/qemu_security_stacked.h b/src/qemu/qemu_security_stacked.h deleted file mode 100644 index 07f76d5..0000000 --- a/src/qemu/qemu_security_stacked.h +++ /dev/null @@ -1,22 +0,0 @@ -/* - * Copyright (C) 2010 Red Hat, Inc. - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * QEMU stacked security driver - */ - -#include "security/security_driver.h" -#include "qemu_conf.h" - -#ifndef __QEMU_SECURITY_STACKED -# define __QEMU_SECURITY_STACKED - -extern virSecurityDriver qemuStackedSecurityDriver; - -void qemuSecurityStackedSetDriver(struct qemud_driver *driver); - -#endif /* __QEMU_SECURITY_DAC */ diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index b43c4ac..37cd928 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -1,4 +1,3 @@ - /* * AppArmor security driver for libvirt * Copyright (C) 2009-2010 Canonical Ltd. @@ -28,7 +27,6 @@ #include "internal.h" -#include "security_driver.h" #include "security_apparmor.h" #include "util.h" #include "memory.h" @@ -47,7 +45,7 @@ /* Data structure to pass to *FileIterate so we have everything we need */ struct SDPDOP { - virSecurityDriverPtr drv; + virSecurityManagerPtr mgr; virDomainObjPtr vm; }; @@ -158,7 +156,7 @@ profile_status_file(const char *str) * load (add) a profile. Will create one if necessary */ static int -load_profile(virSecurityDriverPtr drv, +load_profile(virSecurityManagerPtr mgr, const char *profile, virDomainObjPtr vm, const char *fn, @@ -169,7 +167,7 @@ load_profile(virSecurityDriverPtr drv, char *xml = NULL; int pipefd[2]; pid_t child; - const char *probe = virSecurityDriverGetAllowDiskFormatProbing(drv) + const char *probe = virSecurityManagerGetAllowDiskFormatProbing(mgr) ? "1" : "0"; if (pipe(pipefd) < -1) { @@ -300,7 +298,7 @@ cleanup: * NULL. */ static int -reload_profile(virSecurityDriverPtr drv, +reload_profile(virSecurityManagerPtr mgr, virDomainObjPtr vm, const char *fn, bool append) @@ -317,7 +315,7 @@ reload_profile(virSecurityDriverPtr drv, /* Update the profile only if it is loaded */ if (profile_loaded(secdef->imagelabel) >= 0) { - if (load_profile(drv, secdef->imagelabel, vm, fn, append) < 0) { + if (load_profile(mgr, secdef->imagelabel, vm, fn, append) < 0) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("cannot update AppArmor profile " "\'%s\'"), @@ -340,7 +338,7 @@ AppArmorSetSecurityUSBLabel(usbDevice *dev ATTRIBUTE_UNUSED, struct SDPDOP *ptr = opaque; virDomainObjPtr vm = ptr->vm; - if (reload_profile(ptr->drv, vm, file, true) < 0) { + if (reload_profile(ptr->mgr, vm, file, true) < 0) { const virSecurityLabelDefPtr secdef = &vm->def->seclabel; virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("cannot update AppArmor profile " @@ -358,7 +356,7 @@ AppArmorSetSecurityPCILabel(pciDevice *dev ATTRIBUTE_UNUSED, struct SDPDOP *ptr = opaque; virDomainObjPtr vm = ptr->vm; - if (reload_profile(ptr->drv, vm, file, true) < 0) { + if (reload_profile(ptr->mgr, vm, file, true) < 0) { const virSecurityLabelDefPtr secdef = &vm->def->seclabel; virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("cannot update AppArmor profile " @@ -371,7 +369,7 @@ AppArmorSetSecurityPCILabel(pciDevice *dev ATTRIBUTE_UNUSED, /* Called on libvirtd startup to see if AppArmor is available */ static int -AppArmorSecurityDriverProbe(void) +AppArmorSecurityManagerProbe(void) { char *template = NULL; int rc = SECURITY_DRIVER_DISABLE; @@ -403,21 +401,37 @@ AppArmorSecurityDriverProbe(void) * currently not used. */ static int -AppArmorSecurityDriverOpen(virSecurityDriverPtr drv, - bool allowDiskFormatProbing) +AppArmorSecurityManagerOpen(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int +AppArmorSecurityManagerClose(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) { - virSecurityDriverSetDOI(drv, SECURITY_APPARMOR_VOID_DOI); - virSecurityDriverSetAllowDiskFormatProbing(drv, allowDiskFormatProbing); return 0; } +static const char * +AppArmorSecurityManagerGetModel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + return SECURITY_APPARMOR_NAME; +} + +static const char * +AppArmorSecurityManagerGetDOI(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + return SECURITY_APPARMOR_VOID_DOI; +} + + /* Currently called in qemudStartVMDaemon to setup a 'label'. We look for and * use a profile based on the UUID, otherwise create one based on a template. * Keep in mind that this is called on 'start' with RestoreSecurityLabel being * called on shutdown. */ static int -AppArmorGenSecurityLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, +AppArmorGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainObjPtr vm) { int rc = -1; @@ -472,7 +486,7 @@ AppArmorGenSecurityLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, } static int -AppArmorSetSecurityAllLabel(virSecurityDriverPtr drv, +AppArmorSetSecurityAllLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm, const char *stdin_path) { if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_STATIC) @@ -480,7 +494,7 @@ AppArmorSetSecurityAllLabel(virSecurityDriverPtr drv, /* if the profile is not already loaded, then load one */ if (profile_loaded(vm->def->seclabel.label) < 0) { - if (load_profile(drv, vm->def->seclabel.label, vm, stdin_path, + if (load_profile(mgr, vm->def->seclabel.label, vm, stdin_path, false) < 0) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("cannot generate AppArmor profile " @@ -496,7 +510,7 @@ AppArmorSetSecurityAllLabel(virSecurityDriverPtr drv, * running. */ static int -AppArmorGetSecurityProcessLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, +AppArmorGetSecurityProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainObjPtr vm, virSecurityLabelPtr sec) { @@ -530,7 +544,7 @@ AppArmorGetSecurityProcessLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, * more details. Currently called via qemudShutdownVMDaemon. */ static int -AppArmorReleaseSecurityLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, +AppArmorReleaseSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainObjPtr vm) { const virSecurityLabelDefPtr secdef = &vm->def->seclabel; @@ -544,7 +558,7 @@ AppArmorReleaseSecurityLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, static int -AppArmorRestoreSecurityAllLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, +AppArmorRestoreSecurityAllLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainObjPtr vm, int migrated ATTRIBUTE_UNUSED) { @@ -565,7 +579,7 @@ AppArmorRestoreSecurityAllLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, * LOCALSTATEDIR/log/libvirt/qemu/<vm name>.log */ static int -AppArmorSetSecurityProcessLabel(virSecurityDriverPtr drv, virDomainObjPtr vm) +AppArmorSetSecurityProcessLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm) { const virSecurityLabelDefPtr secdef = &vm->def->seclabel; int rc = -1; @@ -574,12 +588,12 @@ AppArmorSetSecurityProcessLabel(virSecurityDriverPtr drv, virDomainObjPtr vm) if ((profile_name = get_profile_name(vm)) == NULL) return rc; - if (STRNEQ(drv->name, secdef->model)) { + if (STRNEQ(virSecurityManagerGetModel(mgr), secdef->model)) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " "\'%s\' model configured for domain, but " "hypervisor driver is \'%s\'."), - secdef->model, drv->name); + secdef->model, virSecurityManagerGetModel(mgr)); if (use_apparmor() > 0) goto clean; } @@ -597,19 +611,33 @@ AppArmorSetSecurityProcessLabel(virSecurityDriverPtr drv, virDomainObjPtr vm) return rc; } +static int +AppArmorSetSecuritySocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int +AppArmorClearSecuritySocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) +{ + return 0; +} + /* Called when hotplugging */ static int -AppArmorRestoreSecurityImageLabel(virSecurityDriverPtr drv, +AppArmorRestoreSecurityImageLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm, virDomainDiskDefPtr disk ATTRIBUTE_UNUSED) { - return reload_profile(drv, vm, NULL, false); + return reload_profile(mgr, vm, NULL, false); } /* Called when hotplugging */ static int -AppArmorSetSecurityImageLabel(virSecurityDriverPtr drv, +AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm, virDomainDiskDefPtr disk) { const virSecurityLabelDefPtr secdef = &vm->def->seclabel; @@ -635,7 +663,7 @@ AppArmorSetSecurityImageLabel(virSecurityDriverPtr drv, /* update the profile only if it is loaded */ if (profile_loaded(secdef->imagelabel) >= 0) { - if (load_profile(drv, secdef->imagelabel, vm, disk->src, + if (load_profile(mgr, secdef->imagelabel, vm, disk->src, false) < 0) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("cannot update AppArmor profile " @@ -654,7 +682,8 @@ AppArmorSetSecurityImageLabel(virSecurityDriverPtr drv, } static int -AppArmorSecurityVerify(virDomainDefPtr def) +AppArmorSecurityVerify(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr def) { const virSecurityLabelDefPtr secdef = &def->seclabel; @@ -670,7 +699,7 @@ AppArmorSecurityVerify(virDomainDefPtr def) } static int -AppArmorReserveSecurityLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, +AppArmorReserveSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainObjPtr vm ATTRIBUTE_UNUSED) { /* NOOP. Nothing to reserve with AppArmor */ @@ -678,7 +707,7 @@ AppArmorReserveSecurityLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, } static int -AppArmorSetSecurityHostdevLabel(virSecurityDriverPtr drv, +AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm, virDomainHostdevDefPtr dev) @@ -698,7 +727,7 @@ AppArmorSetSecurityHostdevLabel(virSecurityDriverPtr drv, if (VIR_ALLOC(ptr) < 0) return -1; - ptr->drv = drv; + ptr->mgr = mgr; ptr->vm = vm; switch (dev->source.subsys.type) { @@ -740,7 +769,7 @@ done: static int -AppArmorRestoreSecurityHostdevLabel(virSecurityDriverPtr drv, +AppArmorRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm, virDomainHostdevDefPtr dev ATTRIBUTE_UNUSED) @@ -749,42 +778,57 @@ AppArmorRestoreSecurityHostdevLabel(virSecurityDriverPtr drv, if (secdef->type == VIR_DOMAIN_SECLABEL_STATIC) return 0; - return reload_profile(drv, vm, NULL, false); + return reload_profile(mgr, vm, NULL, false); } static int -AppArmorSetSavedStateLabel(virSecurityDriverPtr drv, +AppArmorSetSavedStateLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm, const char *savefile) { - return reload_profile(drv, vm, savefile, true); + return reload_profile(mgr, vm, savefile, true); } static int -AppArmorRestoreSavedStateLabel(virSecurityDriverPtr drv, +AppArmorRestoreSavedStateLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm, const char *savefile ATTRIBUTE_UNUSED) { - return reload_profile(drv, vm, NULL, false); + return reload_profile(mgr, vm, NULL, false); } virSecurityDriver virAppArmorSecurityDriver = { - .name = SECURITY_APPARMOR_NAME, - .probe = AppArmorSecurityDriverProbe, - .open = AppArmorSecurityDriverOpen, - .domainSecurityVerify = AppArmorSecurityVerify, - .domainSetSecurityImageLabel = AppArmorSetSecurityImageLabel, - .domainRestoreSecurityImageLabel = AppArmorRestoreSecurityImageLabel, - .domainGenSecurityLabel = AppArmorGenSecurityLabel, - .domainReserveSecurityLabel = AppArmorReserveSecurityLabel, - .domainReleaseSecurityLabel = AppArmorReleaseSecurityLabel, - .domainGetSecurityProcessLabel = AppArmorGetSecurityProcessLabel, - .domainSetSecurityProcessLabel = AppArmorSetSecurityProcessLabel, - .domainRestoreSecurityAllLabel = AppArmorRestoreSecurityAllLabel, - .domainSetSecurityAllLabel = AppArmorSetSecurityAllLabel, - .domainSetSecurityHostdevLabel = AppArmorSetSecurityHostdevLabel, - .domainRestoreSecurityHostdevLabel = AppArmorRestoreSecurityHostdevLabel, - .domainSetSavedStateLabel = AppArmorSetSavedStateLabel, - .domainRestoreSavedStateLabel = AppArmorRestoreSavedStateLabel, + 0, + SECURITY_APPARMOR_NAME, + AppArmorSecurityManagerProbe, + AppArmorSecurityManagerOpen, + AppArmorSecurityManagerClose, + + AppArmorSecurityManagerGetModel, + AppArmorSecurityManagerGetDOI, + + AppArmorSecurityVerify, + + AppArmorSetSecurityImageLabel, + AppArmorRestoreSecurityImageLabel, + + AppArmorSetSecuritySocketLabel, + AppArmorClearSecuritySocketLabel, + + AppArmorGenSecurityLabel, + AppArmorReserveSecurityLabel, + AppArmorReleaseSecurityLabel, + + AppArmorGetSecurityProcessLabel, + AppArmorSetSecurityProcessLabel, + + AppArmorSetSecurityAllLabel, + AppArmorRestoreSecurityAllLabel, + + AppArmorSetSecurityHostdevLabel, + AppArmorRestoreSecurityHostdevLabel, + + AppArmorSetSavedStateLabel, + AppArmorRestoreSavedStateLabel, }; diff --git a/src/security/security_apparmor.h b/src/security/security_apparmor.h index eb7e140..90d9ddb 100644 --- a/src/security/security_apparmor.h +++ b/src/security/security_apparmor.h @@ -14,6 +14,8 @@ #ifndef __VIR_SECURITY_APPARMOR_H__ # define __VIR_SECURITY_APPARMOR_H__ +#include "security_driver.h" + extern virSecurityDriver virAppArmorSecurityDriver; # define AA_PREFIX "libvirt-" diff --git a/src/security/security_dac.c b/src/security/security_dac.c new file mode 100644 index 0000000..b4b9c6e --- /dev/null +++ b/src/security/security_dac.c @@ -0,0 +1,712 @@ +/* + * Copyright (C) 2010 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * QEMU POSIX DAC security driver + */ +#include <config.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> + +#include "security_dac.h" +#include "virterror_internal.h" +#include "util.h" +#include "memory.h" +#include "logging.h" +#include "pci.h" +#include "hostusb.h" +#include "storage_file.h" + +#define VIR_FROM_THIS VIR_FROM_SECURITY + +typedef struct _virSecurityDACData virSecurityDACData; +typedef virSecurityDACData *virSecurityDACDataPtr; + +struct _virSecurityDACData { + uid_t user; + gid_t group; + bool dynamicOwnership; +}; + +void virSecurityDACSetUser(virSecurityManagerPtr mgr, + uid_t user) +{ + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + priv->user = user; +} + +void virSecurityDACSetGroup(virSecurityManagerPtr mgr, + gid_t group) +{ + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + priv->group = group; +} + +void virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr, + bool dynamicOwnership) +{ + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + priv->dynamicOwnership = dynamicOwnership; +} + +static virSecurityDriverStatus +virSecurityDACProbe(void) +{ + return SECURITY_DRIVER_ENABLE; +} + +static int +virSecurityDACOpen(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int +virSecurityDACClose(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + return 0; +} + + +static const char * virSecurityDACGetModel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + return "dac"; +} + +static const char * virSecurityDACGetDOI(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + return "0"; +} + +static int +virSecurityDACSetOwnership(const char *path, int uid, int gid) +{ + VIR_INFO("Setting DAC user and group on '%s' to '%d:%d'", path, uid, gid); + + if (chown(path, uid, gid) < 0) { + struct stat sb; + int chown_errno = errno; + + if (stat(path, &sb) >= 0) { + if (sb.st_uid == uid && + sb.st_gid == gid) { + /* It's alright, there's nothing to change anyway. */ + return 0; + } + } + + if (chown_errno == EOPNOTSUPP) { + VIR_INFO("Setting user and group to '%d:%d' on '%s' not supported by filesystem", + uid, gid, path); + } else if (chown_errno == EPERM) { + VIR_INFO("Setting user and group to '%d:%d' on '%s' not permitted", + uid, gid, path); + } else if (chown_errno == EROFS) { + VIR_INFO("Setting user and group to '%d:%d' on '%s' not possible on readonly filesystem", + uid, gid, path); + } else { + virReportSystemError(chown_errno, + _("unable to set user and group to '%d:%d' on '%s'"), + uid, gid, path); + return -1; + } + } + return 0; +} + +static int +virSecurityDACRestoreSecurityFileLabel(const char *path) +{ + struct stat buf; + int rc = -1; + char *newpath = NULL; + + VIR_INFO("Restoring DAC user and group on '%s'", path); + + if (virFileResolveLink(path, &newpath) < 0) { + virReportSystemError(errno, + _("cannot resolve symlink %s"), path); + goto err; + } + + if (stat(newpath, &buf) != 0) + goto err; + + /* XXX record previous ownership */ + rc = virSecurityDACSetOwnership(newpath, 0, 0); + +err: + VIR_FREE(newpath); + return rc; +} + + +static int +virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, + const char *path, + size_t depth ATTRIBUTE_UNUSED, + void *opaque) +{ + virSecurityManagerPtr mgr = opaque; + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + + return virSecurityDACSetOwnership(path, priv->user, priv->group); +} + + +static int +virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + virDomainDiskDefPtr disk) + +{ + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + + if (!priv->dynamicOwnership) + return 0; + + return virDomainDiskDefForeachPath(disk, + virSecurityManagerGetAllowDiskFormatProbing(mgr), + false, + virSecurityDACSetSecurityFileLabel, + mgr); +} + + +static int +virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + virDomainDiskDefPtr disk, + int migrated) +{ + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + + if (!priv->dynamicOwnership) + return 0; + + /* Don't restore labels on readoly/shared disks, because + * other VMs may still be accessing these + * Alternatively we could iterate over all running + * domains and try to figure out if it is in use, but + * this would not work for clustered filesystems, since + * we can't see running VMs using the file on other nodes + * Safest bet is thus to skip the restore step. + */ + if (disk->readonly || disk->shared) + return 0; + + if (!disk->src) + return 0; + + /* If we have a shared FS & doing migrated, we must not + * change ownership, because that kills access on the + * destination host which is sub-optimal for the guest + * VM's I/O attempts :-) + */ + if (migrated) { + int rc = virStorageFileIsSharedFS(disk->src); + if (rc < 0) + return -1; + if (rc == 1) { + VIR_DEBUG("Skipping image label restore on %s because FS is shared", + disk->src); + return 0; + } + } + + return virSecurityDACRestoreSecurityFileLabel(disk->src); +} + + +static int +virSecurityDACRestoreSecurityImageLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ + return virSecurityDACRestoreSecurityImageLabelInt(mgr, vm, disk, 0); +} + + +static int +virSecurityDACSetSecurityPCILabel(pciDevice *dev ATTRIBUTE_UNUSED, + const char *file, + void *opaque) +{ + virSecurityManagerPtr mgr = opaque; + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + + return virSecurityDACSetOwnership(file, priv->user, priv->group); +} + + +static int +virSecurityDACSetSecurityUSBLabel(usbDevice *dev ATTRIBUTE_UNUSED, + const char *file, + void *opaque) +{ + virSecurityManagerPtr mgr = opaque; + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + + return virSecurityDACSetOwnership(file, priv->user, priv->group); +} + + +static int +virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + virDomainHostdevDefPtr dev) +{ + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int ret = -1; + + if (!priv->dynamicOwnership) + return 0; + + if (dev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + return 0; + + switch (dev->source.subsys.type) { + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: { + usbDevice *usb = usbGetDevice(dev->source.subsys.u.usb.bus, + dev->source.subsys.u.usb.device); + + if (!usb) + goto done; + + ret = usbDeviceFileIterate(usb, virSecurityDACSetSecurityUSBLabel, mgr); + usbFreeDevice(usb); + break; + } + + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: { + pciDevice *pci = pciGetDevice(dev->source.subsys.u.pci.domain, + dev->source.subsys.u.pci.bus, + dev->source.subsys.u.pci.slot, + dev->source.subsys.u.pci.function); + + if (!pci) + goto done; + + ret = pciDeviceFileIterate(pci, virSecurityDACSetSecurityPCILabel, mgr); + pciFreeDevice(pci); + + break; + } + + default: + ret = 0; + break; + } + +done: + return ret; +} + + +static int +virSecurityDACRestoreSecurityPCILabel(pciDevice *dev ATTRIBUTE_UNUSED, + const char *file, + void *opaque ATTRIBUTE_UNUSED) +{ + return virSecurityDACRestoreSecurityFileLabel(file); +} + + +static int +virSecurityDACRestoreSecurityUSBLabel(usbDevice *dev ATTRIBUTE_UNUSED, + const char *file, + void *opaque ATTRIBUTE_UNUSED) +{ + return virSecurityDACRestoreSecurityFileLabel(file); +} + + +static int +virSecurityDACRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + virDomainHostdevDefPtr dev) + +{ + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int ret = -1; + + if (!priv->dynamicOwnership) + return 0; + + if (dev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + return 0; + + switch (dev->source.subsys.type) { + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: { + usbDevice *usb = usbGetDevice(dev->source.subsys.u.usb.bus, + dev->source.subsys.u.usb.device); + + if (!usb) + goto done; + + ret = usbDeviceFileIterate(usb, virSecurityDACRestoreSecurityUSBLabel, mgr); + usbFreeDevice(usb); + + break; + } + + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: { + pciDevice *pci = pciGetDevice(dev->source.subsys.u.pci.domain, + dev->source.subsys.u.pci.bus, + dev->source.subsys.u.pci.slot, + dev->source.subsys.u.pci.function); + + if (!pci) + goto done; + + ret = pciDeviceFileIterate(pci, virSecurityDACRestoreSecurityPCILabel, mgr); + pciFreeDevice(pci); + + break; + } + + default: + ret = 0; + break; + } + +done: + return ret; +} + + +static int +virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, + virDomainChrDefPtr dev) + +{ + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + char *in = NULL, *out = NULL; + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_CHR_TYPE_DEV: + case VIR_DOMAIN_CHR_TYPE_FILE: + ret = virSecurityDACSetOwnership(dev->data.file.path, priv->user, priv->group); + break; + + case VIR_DOMAIN_CHR_TYPE_PIPE: + if ((virAsprintf(&in, "%s.in", dev->data.file.path) < 0) || + (virAsprintf(&out, "%s.out", dev->data.file.path) < 0)) { + virReportOOMError(); + goto done; + } + if ((virSecurityDACSetOwnership(in, priv->user, priv->group) < 0) || + (virSecurityDACSetOwnership(out, priv->user, priv->group) < 0)) + goto done; + ret = 0; + break; + + default: + ret = 0; + break; + } + +done: + VIR_FREE(in); + VIR_FREE(out); + return ret; +} + +static int +virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainChrDefPtr dev) +{ + char *in = NULL, *out = NULL; + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_CHR_TYPE_DEV: + case VIR_DOMAIN_CHR_TYPE_FILE: + ret = virSecurityDACRestoreSecurityFileLabel(dev->data.file.path); + break; + + case VIR_DOMAIN_CHR_TYPE_PIPE: + if ((virAsprintf(&out, "%s.out", dev->data.file.path) < 0) || + (virAsprintf(&in, "%s.in", dev->data.file.path) < 0)) { + virReportOOMError(); + goto done; + } + if ((virSecurityDACRestoreSecurityFileLabel(out) < 0) || + (virSecurityDACRestoreSecurityFileLabel(in) < 0)) + goto done; + ret = 0; + break; + + default: + ret = 0; + break; + } + +done: + VIR_FREE(in); + VIR_FREE(out); + return ret; +} + + +static int +virSecurityDACRestoreChardevCallback(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainChrDefPtr dev, + void *opaque) +{ + virSecurityManagerPtr mgr = opaque; + + return virSecurityDACRestoreChardevLabel(mgr, dev); +} + + +static int +virSecurityDACRestoreSecurityAllLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + int migrated) +{ + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int i; + int rc = 0; + + if (!priv->dynamicOwnership) + return 0; + + + VIR_DEBUG("Restoring security label on %s migrated=%d", + vm->def->name, migrated); + + for (i = 0 ; i < vm->def->nhostdevs ; i++) { + if (virSecurityDACRestoreSecurityHostdevLabel(mgr, + vm, + vm->def->hostdevs[i]) < 0) + rc = -1; + } + for (i = 0 ; i < vm->def->ndisks ; i++) { + if (virSecurityDACRestoreSecurityImageLabelInt(mgr, + vm, + vm->def->disks[i], + migrated) < 0) + rc = -1; + } + + if (virDomainChrDefForeach(vm->def, + false, + virSecurityDACRestoreChardevCallback, + vm) < 0) + rc = -1; + + if (vm->def->os.kernel && + virSecurityDACRestoreSecurityFileLabel(vm->def->os.kernel) < 0) + rc = -1; + + if (vm->def->os.initrd && + virSecurityDACRestoreSecurityFileLabel(vm->def->os.initrd) < 0) + rc = -1; + + return rc; +} + + +static int +virSecurityDACSetChardevCallback(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainChrDefPtr dev, + void *opaque) +{ + virSecurityManagerPtr mgr = opaque; + + return virSecurityDACSetChardevLabel(mgr, dev); +} + + +static int +virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + const char *stdin_path ATTRIBUTE_UNUSED) +{ + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int i; + + if (!priv->dynamicOwnership) + return 0; + + for (i = 0 ; i < vm->def->ndisks ; i++) { + /* XXX fixme - we need to recursively label the entriy tree :-( */ + if (vm->def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_DIR) + continue; + if (virSecurityDACSetSecurityImageLabel(mgr, + vm, + vm->def->disks[i]) < 0) + return -1; + } + for (i = 0 ; i < vm->def->nhostdevs ; i++) { + if (virSecurityDACSetSecurityHostdevLabel(mgr, + vm, + vm->def->hostdevs[i]) < 0) + return -1; + } + + if (virDomainChrDefForeach(vm->def, + true, + virSecurityDACSetChardevCallback, + vm) < 0) + return -1; + + if (vm->def->os.kernel && + virSecurityDACSetOwnership(vm->def->os.kernel, + priv->user, + priv->group) < 0) + return -1; + + if (vm->def->os.initrd && + virSecurityDACSetOwnership(vm->def->os.initrd, + priv->user, + priv->group) < 0) + return -1; + + return 0; +} + + +static int +virSecurityDACSetSavedStateLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + const char *savefile) +{ + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + + return virSecurityDACSetOwnership(savefile, priv->user, priv->group); +} + + +static int +virSecurityDACRestoreSavedStateLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + const char *savefile) +{ + return virSecurityDACRestoreSecurityFileLabel(savefile); +} + + +static int +virSecurityDACSetProcessLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm ATTRIBUTE_UNUSED) +{ + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + + DEBUG("Dropping privileges of VM to %d:%d", priv->user, priv->group); + + if (priv->group) { + if (setregid(priv->group, priv->group) < 0) { + virReportSystemError(errno, + _("cannot change to '%d' group"), + priv->group); + return -1; + } + } + if (priv->user) { + if (setreuid(priv->user, priv->user) < 0) { + virReportSystemError(errno, + _("cannot change to '%d' user"), + priv->user); + return -1; + } + } + + return 0; +} + + +static int +virSecurityDACVerify(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr def ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int +virSecurityDACGenLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int +virSecurityDACReleaseLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int +virSecurityDACReserveLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int +virSecurityDACGetProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + virSecurityLabelPtr seclabel ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int +virSecurityDACSetSocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) +{ + return 0; +} + + +static int +virSecurityDACClearSocketLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) +{ + return 0; +} + + +virSecurityDriver virSecurityDriverDAC = { + sizeof(virSecurityDACData), + "virDAC", + + virSecurityDACProbe, + virSecurityDACOpen, + virSecurityDACClose, + + virSecurityDACGetModel, + virSecurityDACGetDOI, + + virSecurityDACVerify, + + virSecurityDACSetSecurityImageLabel, + virSecurityDACRestoreSecurityImageLabel, + + virSecurityDACSetSocketLabel, + virSecurityDACClearSocketLabel, + + virSecurityDACGenLabel, + virSecurityDACReserveLabel, + virSecurityDACReleaseLabel, + + virSecurityDACGetProcessLabel, + virSecurityDACSetProcessLabel, + + virSecurityDACSetSecurityAllLabel, + virSecurityDACRestoreSecurityAllLabel, + + virSecurityDACSetSecurityHostdevLabel, + virSecurityDACRestoreSecurityHostdevLabel, + + virSecurityDACSetSavedStateLabel, + virSecurityDACRestoreSavedStateLabel, +}; diff --git a/src/security/security_dac.h b/src/security/security_dac.h new file mode 100644 index 0000000..b690236 --- /dev/null +++ b/src/security/security_dac.h @@ -0,0 +1,27 @@ +/* + * Copyright (C) 2010 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * POSIX DAC security driver + */ + +#include "security_driver.h" + +#ifndef __VIR_SECURITY_DAC +# define __VIR_SECURITY_DAC + +extern virSecurityDriver virSecurityDriverDAC; + +void virSecurityDACSetUser(virSecurityManagerPtr mgr, + uid_t user); +void virSecurityDACSetGroup(virSecurityManagerPtr mgr, + gid_t group); + +void virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr, + bool dynamic); + +#endif /* __VIR_SECURITY_DAC */ diff --git a/src/security/security_driver.c b/src/security/security_driver.c index 9e32fa4..6d75dcc 100644 --- a/src/security/security_driver.c +++ b/src/security/security_driver.c @@ -24,116 +24,52 @@ # include "security_apparmor.h" #endif +#include "security_nop.h" + static virSecurityDriverPtr security_drivers[] = { #ifdef WITH_SECDRIVER_SELINUX - &virSELinuxSecurityDriver, + &virSecurityDriverSELinux, #endif #ifdef WITH_SECDRIVER_APPARMOR &virAppArmorSecurityDriver, #endif - NULL + &virSecurityDriverNop, /* Must always be last, since it will always probe */ }; -int -virSecurityDriverVerify(virDomainDefPtr def) -{ - unsigned int i; - const virSecurityLabelDefPtr secdef = &def->seclabel; - - if (!secdef->model || - STREQ(secdef->model, "none")) - return 0; - - for (i = 0; security_drivers[i] != NULL ; i++) { - if (STREQ(security_drivers[i]->name, secdef->model)) { - return security_drivers[i]->domainSecurityVerify(def); - } - } - virSecurityReportError(VIR_ERR_XML_ERROR, - _("invalid security model '%s'"), secdef->model); - return -1; -} - -int -virSecurityDriverStartup(virSecurityDriverPtr *drv, - const char *name, - bool allowDiskFormatProbing) +virSecurityDriverPtr virSecurityDriverLookup(const char *name) { - unsigned int i; - - if (name && STREQ(name, "none")) - return -2; + virSecurityDriverPtr drv = NULL; + int i; - for (i = 0; security_drivers[i] != NULL ; i++) { + for (i = 0; i < ARRAY_CARDINALITY(security_drivers) ; i++) { virSecurityDriverPtr tmp = security_drivers[i]; - if (name && STRNEQ(tmp->name, name)) - continue; - - switch (tmp->probe()) { - case SECURITY_DRIVER_ENABLE: - virSecurityDriverInit(tmp); - if (tmp->open(tmp, allowDiskFormatProbing) == -1) { - return -1; - } else { - *drv = tmp; - return 0; + if (name) { + if (STREQ(tmp->name, name)) { + drv = tmp; + break; } - break; + } else { + switch (tmp->probe()) { + case SECURITY_DRIVER_ENABLE: + drv = tmp; + break; - case SECURITY_DRIVER_DISABLE: - break; + case SECURITY_DRIVER_DISABLE: + break; - default: - return -1; + default: + return NULL; + } } } - return -2; -} - -/* - * Helpers - */ -void -virSecurityDriverInit(virSecurityDriverPtr drv) -{ - memset(&drv->_private, 0, sizeof drv->_private); -} -int -virSecurityDriverSetDOI(virSecurityDriverPtr drv, - const char *doi) -{ - if (strlen(doi) >= VIR_SECURITY_DOI_BUFLEN) { + if (!drv) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, - _("%s: DOI \'%s\' is " - "longer than the maximum allowed length of %d"), - __func__, doi, VIR_SECURITY_DOI_BUFLEN - 1); - return -1; + "Security driver %s not found", NULLSTR(name)); + return NULL; } - strcpy(drv->_private.doi, doi); - return 0; -} - -const char * -virSecurityDriverGetDOI(virSecurityDriverPtr drv) -{ - return drv->_private.doi; -} -const char * -virSecurityDriverGetModel(virSecurityDriverPtr drv) -{ - return drv->name; + return drv; } -void virSecurityDriverSetAllowDiskFormatProbing(virSecurityDriverPtr drv, - bool allowDiskFormatProbing) -{ - drv->_private.allowDiskFormatProbing = allowDiskFormatProbing; -} - -bool virSecurityDriverGetAllowDiskFormatProbing(virSecurityDriverPtr drv) -{ - return drv->_private.allowDiskFormatProbing; -} diff --git a/src/security/security_driver.h b/src/security/security_driver.h index d768f32..e5a8d41 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -16,6 +16,8 @@ # include "internal.h" # include "domain_conf.h" +# include "security_manager.h" + /* * Return values for security driver probing: the driver will determine * whether it should be enabled or disabled. @@ -29,104 +31,91 @@ typedef enum { typedef struct _virSecurityDriver virSecurityDriver; typedef virSecurityDriver *virSecurityDriverPtr; -typedef struct _virSecurityDriverState virSecurityDriverState; -typedef virSecurityDriverState *virSecurityDriverStatePtr; - typedef virSecurityDriverStatus (*virSecurityDriverProbe) (void); -typedef int (*virSecurityDriverOpen) (virSecurityDriverPtr drv, - bool allowDiskFormatProbing); -typedef int (*virSecurityDomainRestoreImageLabel) (virSecurityDriverPtr drv, +typedef int (*virSecurityDriverOpen) (virSecurityManagerPtr mgr); +typedef int (*virSecurityDriverClose) (virSecurityManagerPtr mgr); + +typedef const char *(*virSecurityDriverGetModel) (virSecurityManagerPtr mgr); +typedef const char *(*virSecurityDriverGetDOI) (virSecurityManagerPtr mgr); + +typedef int (*virSecurityDomainRestoreImageLabel) (virSecurityManagerPtr mgr, virDomainObjPtr vm, virDomainDiskDefPtr disk); -typedef int (*virSecurityDomainSetSocketLabel) (virSecurityDriverPtr drv, +typedef int (*virSecurityDomainSetSocketLabel) (virSecurityManagerPtr mgr, virDomainObjPtr vm); -typedef int (*virSecurityDomainClearSocketLabel)(virSecurityDriverPtr drv, +typedef int (*virSecurityDomainClearSocketLabel)(virSecurityManagerPtr mgr, virDomainObjPtr vm); -typedef int (*virSecurityDomainSetImageLabel) (virSecurityDriverPtr drv, +typedef int (*virSecurityDomainSetImageLabel) (virSecurityManagerPtr mgr, virDomainObjPtr vm, virDomainDiskDefPtr disk); -typedef int (*virSecurityDomainRestoreHostdevLabel) (virSecurityDriverPtr drv, +typedef int (*virSecurityDomainRestoreHostdevLabel) (virSecurityManagerPtr mgr, virDomainObjPtr vm, virDomainHostdevDefPtr dev); -typedef int (*virSecurityDomainSetHostdevLabel) (virSecurityDriverPtr drv, +typedef int (*virSecurityDomainSetHostdevLabel) (virSecurityManagerPtr mgr, virDomainObjPtr vm, virDomainHostdevDefPtr dev); -typedef int (*virSecurityDomainSetSavedStateLabel) (virSecurityDriverPtr drv, +typedef int (*virSecurityDomainSetSavedStateLabel) (virSecurityManagerPtr mgr, virDomainObjPtr vm, const char *savefile); -typedef int (*virSecurityDomainRestoreSavedStateLabel) (virSecurityDriverPtr drv, +typedef int (*virSecurityDomainRestoreSavedStateLabel) (virSecurityManagerPtr mgr, virDomainObjPtr vm, const char *savefile); -typedef int (*virSecurityDomainGenLabel) (virSecurityDriverPtr drv, +typedef int (*virSecurityDomainGenLabel) (virSecurityManagerPtr mgr, virDomainObjPtr sec); -typedef int (*virSecurityDomainReserveLabel) (virSecurityDriverPtr drv, +typedef int (*virSecurityDomainReserveLabel) (virSecurityManagerPtr mgr, virDomainObjPtr sec); -typedef int (*virSecurityDomainReleaseLabel) (virSecurityDriverPtr drv, +typedef int (*virSecurityDomainReleaseLabel) (virSecurityManagerPtr mgr, virDomainObjPtr sec); -typedef int (*virSecurityDomainSetAllLabel) (virSecurityDriverPtr drv, +typedef int (*virSecurityDomainSetAllLabel) (virSecurityManagerPtr mgr, virDomainObjPtr sec, const char *stdin_path); -typedef int (*virSecurityDomainRestoreAllLabel) (virSecurityDriverPtr drv, +typedef int (*virSecurityDomainRestoreAllLabel) (virSecurityManagerPtr mgr, virDomainObjPtr vm, int migrated); -typedef int (*virSecurityDomainGetProcessLabel) (virSecurityDriverPtr drv, +typedef int (*virSecurityDomainGetProcessLabel) (virSecurityManagerPtr mgr, virDomainObjPtr vm, virSecurityLabelPtr sec); -typedef int (*virSecurityDomainSetProcessLabel) (virSecurityDriverPtr drv, +typedef int (*virSecurityDomainSetProcessLabel) (virSecurityManagerPtr mgr, virDomainObjPtr vm); -typedef int (*virSecurityDomainSecurityVerify) (virDomainDefPtr def); +typedef int (*virSecurityDomainSecurityVerify) (virSecurityManagerPtr mgr, + virDomainDefPtr def); + struct _virSecurityDriver { + size_t privateDataLen; const char *name; virSecurityDriverProbe probe; virSecurityDriverOpen open; + virSecurityDriverClose close; + + virSecurityDriverGetModel getModel; + virSecurityDriverGetDOI getDOI; + virSecurityDomainSecurityVerify domainSecurityVerify; + + virSecurityDomainSetImageLabel domainSetSecurityImageLabel; virSecurityDomainRestoreImageLabel domainRestoreSecurityImageLabel; + virSecurityDomainSetSocketLabel domainSetSecuritySocketLabel; virSecurityDomainClearSocketLabel domainClearSecuritySocketLabel; - virSecurityDomainSetImageLabel domainSetSecurityImageLabel; + virSecurityDomainGenLabel domainGenSecurityLabel; virSecurityDomainReserveLabel domainReserveSecurityLabel; virSecurityDomainReleaseLabel domainReleaseSecurityLabel; + virSecurityDomainGetProcessLabel domainGetSecurityProcessLabel; virSecurityDomainSetProcessLabel domainSetSecurityProcessLabel; + virSecurityDomainSetAllLabel domainSetSecurityAllLabel; virSecurityDomainRestoreAllLabel domainRestoreSecurityAllLabel; - virSecurityDomainRestoreHostdevLabel domainRestoreSecurityHostdevLabel; + virSecurityDomainSetHostdevLabel domainSetSecurityHostdevLabel; + virSecurityDomainRestoreHostdevLabel domainRestoreSecurityHostdevLabel; + virSecurityDomainSetSavedStateLabel domainSetSavedStateLabel; virSecurityDomainRestoreSavedStateLabel domainRestoreSavedStateLabel; - - /* - * This is internally managed driver state and should only be accessed - * via helpers below. - */ - struct { - char doi[VIR_SECURITY_DOI_BUFLEN]; - bool allowDiskFormatProbing; - } _private; }; -/* Global methods */ -int virSecurityDriverStartup(virSecurityDriverPtr *drv, - const char *name, - bool allowDiskFormatProbing); - -int -virSecurityDriverVerify(virDomainDefPtr def); - -# define virSecurityReportError(code, ...) \ - virReportErrorHelper(NULL, VIR_FROM_SECURITY, code, __FILE__, \ - __FUNCTION__, __LINE__, __VA_ARGS__) - -/* Helpers */ -void virSecurityDriverInit(virSecurityDriverPtr drv); -int virSecurityDriverSetDOI(virSecurityDriverPtr drv, - const char *doi); -void virSecurityDriverSetAllowDiskFormatProbing(virSecurityDriverPtr drv, - bool allowDiskFormatProbing); -const char *virSecurityDriverGetDOI(virSecurityDriverPtr drv); -const char *virSecurityDriverGetModel(virSecurityDriverPtr drv); -bool virSecurityDriverGetAllowDiskFormatProbing(virSecurityDriverPtr drv); +virSecurityDriverPtr virSecurityDriverLookup(const char *name); #endif /* __VIR_SECURITY_H__ */ diff --git a/src/security/security_manager.c b/src/security/security_manager.c new file mode 100644 index 0000000..7ab6e37 --- /dev/null +++ b/src/security/security_manager.c @@ -0,0 +1,291 @@ + +#include <config.h> + + +#include "security_driver.h" +#include "security_stack.h" +#include "security_dac.h" +#include "virterror_internal.h" +#include "memory.h" +#include "logging.h" + +#define VIR_FROM_THIS VIR_FROM_SECURITY + + +struct _virSecurityManager { + virSecurityDriverPtr drv; + bool allowDiskFormatProbing; +}; + +static virSecurityManagerPtr virSecurityManagerNewDriver(virSecurityDriverPtr drv, + bool allowDiskFormatProbing) +{ + virSecurityManagerPtr mgr; + + if (VIR_ALLOC_VAR(mgr, char, drv->privateDataLen) < 0) { + virReportOOMError(); + return NULL; + } + + mgr->drv = drv; + mgr->allowDiskFormatProbing = allowDiskFormatProbing; + + if (drv->open(mgr) < 0) { + virSecurityManagerFree(mgr); + return NULL; + } + + return mgr; +} + +virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary, + virSecurityManagerPtr secondary) +{ + virSecurityManagerPtr mgr = + virSecurityManagerNewDriver(&virSecurityDriverStack, + virSecurityManagerGetAllowDiskFormatProbing(primary)); + + virSecurityStackSetPrimary(mgr, primary); + virSecurityStackSetSecondary(mgr, secondary); + + return mgr; +} + +virSecurityManagerPtr virSecurityManagerNewDAC(uid_t user, + gid_t group, + bool allowDiskFormatProbing, + bool dynamicOwnership) +{ + virSecurityManagerPtr mgr = + virSecurityManagerNewDriver(&virSecurityDriverDAC, + allowDiskFormatProbing); + + virSecurityDACSetUser(mgr, user); + virSecurityDACSetGroup(mgr, group); + virSecurityDACSetDynamicOwnership(mgr, dynamicOwnership); + + return mgr; +} + +virSecurityManagerPtr virSecurityManagerNew(const char *name, + bool allowDiskFormatProbing) +{ + virSecurityDriverPtr drv = virSecurityDriverLookup(name); + if (!drv) + return NULL; + + return virSecurityManagerNewDriver(drv, allowDiskFormatProbing); +} + + +void *virSecurityManagerGetPrivateData(virSecurityManagerPtr mgr) +{ + return mgr + sizeof(mgr); +} + + +void virSecurityManagerFree(virSecurityManagerPtr mgr) +{ + if (!mgr) + return; + + if (mgr->drv->close) + mgr->drv->close(mgr); + + VIR_FREE(mgr); +} + +const char * +virSecurityManagerGetDOI(virSecurityManagerPtr mgr) +{ + if (mgr->drv->getDOI) + return mgr->drv->getDOI(mgr); + + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return NULL; +} + +const char * +virSecurityManagerGetModel(virSecurityManagerPtr mgr) +{ + if (mgr->drv->getModel) + return mgr->drv->getModel(mgr); + + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return NULL; +} + +bool virSecurityManagerGetAllowDiskFormatProbing(virSecurityManagerPtr mgr) +{ + return mgr->allowDiskFormatProbing; +} + +int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ + if (mgr->drv->domainRestoreSecurityImageLabel) + return mgr->drv->domainRestoreSecurityImageLabel(mgr, vm, disk); + + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; +} + +int virSecurityManagerSetSocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) +{ + if (mgr->drv->domainSetSecuritySocketLabel) + return mgr->drv->domainSetSecuritySocketLabel(mgr, vm); + + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; +} + +int virSecurityManagerClearSocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) +{ + if (mgr->drv->domainClearSecuritySocketLabel) + return mgr->drv->domainClearSecuritySocketLabel(mgr, vm); + + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; +} + +int virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ + if (mgr->drv->domainSetSecurityImageLabel) + return mgr->drv->domainSetSecurityImageLabel(mgr, vm, disk); + + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; +} + +int virSecurityManagerRestoreHostdevLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + virDomainHostdevDefPtr dev) +{ + if (mgr->drv->domainRestoreSecurityHostdevLabel) + return mgr->drv->domainRestoreSecurityHostdevLabel(mgr, vm, dev); + + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; +} + +int virSecurityManagerSetHostdevLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + virDomainHostdevDefPtr dev) +{ + if (mgr->drv->domainSetSecurityHostdevLabel) + return mgr->drv->domainSetSecurityHostdevLabel(mgr, vm, dev); + + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; +} + +int virSecurityManagerSetSavedStateLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + const char *savefile) +{ + if (mgr->drv->domainSetSavedStateLabel) + return mgr->drv->domainSetSavedStateLabel(mgr, vm, savefile); + + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; +} + +int virSecurityManagerRestoreSavedStateLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + const char *savefile) +{ + if (mgr->drv->domainRestoreSavedStateLabel) + return mgr->drv->domainRestoreSavedStateLabel(mgr, vm, savefile); + + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; +} + +int virSecurityManagerGenLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) +{ + if (mgr->drv->domainGenSecurityLabel) + return mgr->drv->domainGenSecurityLabel(mgr, vm); + + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; +} + +int virSecurityManagerReserveLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) +{ + if (mgr->drv->domainReserveSecurityLabel) + return mgr->drv->domainReserveSecurityLabel(mgr, vm); + + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; +} + +int virSecurityManagerReleaseLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) +{ + if (mgr->drv->domainReleaseSecurityLabel) + return mgr->drv->domainReleaseSecurityLabel(mgr, vm); + + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; +} + +int virSecurityManagerSetAllLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + const char *stdin_path) +{ + if (mgr->drv->domainSetSecurityAllLabel) + return mgr->drv->domainSetSecurityAllLabel(mgr, vm, stdin_path); + + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; +} + +int virSecurityManagerRestoreAllLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + int migrated) +{ + if (mgr->drv->domainRestoreSecurityAllLabel) + return mgr->drv->domainRestoreSecurityAllLabel(mgr, vm, migrated); + + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; +} + +int virSecurityManagerGetProcessLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + virSecurityLabelPtr sec) +{ + if (mgr->drv->domainGetSecurityProcessLabel) + return mgr->drv->domainGetSecurityProcessLabel(mgr, vm, sec); + + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; +} + +int virSecurityManagerSetProcessLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) +{ + if (mgr->drv->domainSetSecurityProcessLabel) + return mgr->drv->domainSetSecurityProcessLabel(mgr, vm); + + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; +} + +int virSecurityManagerVerify(virSecurityManagerPtr mgr, + virDomainDefPtr def) +{ + if (mgr->drv->domainSecurityVerify) + return mgr->drv->domainSecurityVerify(mgr, def); + + virSecurityReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; +} + diff --git a/src/security/security_manager.h b/src/security/security_manager.h new file mode 100644 index 0000000..c0ef84f --- /dev/null +++ b/src/security/security_manager.h @@ -0,0 +1,74 @@ + +#ifndef VIR_SECURITY_MANAGER_H__ +#define VIR_SECURITY_MANAGER_H__ + +# define virSecurityReportError(code, ...) \ + virReportErrorHelper(NULL, VIR_FROM_SECURITY, code, __FILE__, \ + __FUNCTION__, __LINE__, __VA_ARGS__) + + +typedef struct _virSecurityManager virSecurityManager; +typedef virSecurityManager *virSecurityManagerPtr; + +virSecurityManagerPtr virSecurityManagerNew(const char *name, + bool allowDiskFormatProbing); + +virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary, + virSecurityManagerPtr secondary); + +virSecurityManagerPtr virSecurityManagerNewDAC(uid_t user, + gid_t group, + bool allowDiskFormatProbing, + bool dynamicOwnership); + +void *virSecurityManagerGetPrivateData(virSecurityManagerPtr mgr); + +void virSecurityManagerFree(virSecurityManagerPtr mgr); + +const char *virSecurityManagerGetDOI(virSecurityManagerPtr mgr); +const char *virSecurityManagerGetModel(virSecurityManagerPtr mgr); +bool virSecurityManagerGetAllowDiskFormatProbing(virSecurityManagerPtr mgr); + +int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + virDomainDiskDefPtr disk); +int virSecurityManagerSetSocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm); +int virSecurityManagerClearSocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm); +int virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + virDomainDiskDefPtr disk); +int virSecurityManagerRestoreHostdevLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + virDomainHostdevDefPtr dev); +int virSecurityManagerSetHostdevLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + virDomainHostdevDefPtr dev); +int virSecurityManagerSetSavedStateLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + const char *savefile); +int virSecurityManagerRestoreSavedStateLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + const char *savefile); +int virSecurityManagerGenLabel(virSecurityManagerPtr mgr, + virDomainObjPtr sec); +int virSecurityManagerReserveLabel(virSecurityManagerPtr mgr, + virDomainObjPtr sec); +int virSecurityManagerReleaseLabel(virSecurityManagerPtr mgr, + virDomainObjPtr sec); +int virSecurityManagerSetAllLabel(virSecurityManagerPtr mgr, + virDomainObjPtr sec, + const char *stdin_path); +int virSecurityManagerRestoreAllLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + int migrated); +int virSecurityManagerGetProcessLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + virSecurityLabelPtr sec); +int virSecurityManagerSetProcessLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm); +int virSecurityManagerVerify(virSecurityManagerPtr mgr, + virDomainDefPtr def); + +#endif /* VIR_SECURITY_MANAGER_H__ */ diff --git a/src/security/security_nop.c b/src/security/security_nop.c new file mode 100644 index 0000000..947cf55 --- /dev/null +++ b/src/security/security_nop.c @@ -0,0 +1,168 @@ + + +#include <config.h> + +#include "security_nop.h" + +static virSecurityDriverStatus virSecurityDriverProbeNop(void) +{ + return SECURITY_DRIVER_ENABLE; +} + +static int virSecurityDriverOpenNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int virSecurityDriverCloseNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + return 0; +} + +static const char * virSecurityDriverGetModelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + return "none"; +} + +static const char * virSecurityDriverGetDOINop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + return "0"; +} + +static int virSecurityDomainRestoreImageLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + virDomainDiskDefPtr disk ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int virSecurityDomainSetSocketLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int virSecurityDomainClearSocketLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int virSecurityDomainSetImageLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + virDomainDiskDefPtr disk ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int virSecurityDomainRestoreHostdevLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + virDomainHostdevDefPtr dev ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int virSecurityDomainSetHostdevLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + virDomainHostdevDefPtr dev ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int virSecurityDomainSetSavedStateLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + const char *savefile ATTRIBUTE_UNUSED) +{ + return 0; +} +static int virSecurityDomainRestoreSavedStateLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + const char *savefile ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int virSecurityDomainGenLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr sec ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int virSecurityDomainReserveLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr sec ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int virSecurityDomainReleaseLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr sec ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int virSecurityDomainSetAllLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr sec ATTRIBUTE_UNUSED, + const char *stdin_path ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int virSecurityDomainRestoreAllLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + int migrated ATTRIBUTE_UNUSED) +{ + return 0; +} +static int virSecurityDomainGetProcessLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + virSecurityLabelPtr sec ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int virSecurityDomainSetProcessLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int virSecurityDomainVerifyNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr def ATTRIBUTE_UNUSED) +{ + return 0; +} + +virSecurityDriver virSecurityDriverNop = { + 0, + "nop", + virSecurityDriverProbeNop, + virSecurityDriverOpenNop, + virSecurityDriverCloseNop, + + virSecurityDriverGetModelNop, + virSecurityDriverGetDOINop, + + virSecurityDomainVerifyNop, + + virSecurityDomainSetImageLabelNop, + virSecurityDomainRestoreImageLabelNop, + + virSecurityDomainSetSocketLabelNop, + virSecurityDomainClearSocketLabelNop, + + virSecurityDomainGenLabelNop, + virSecurityDomainReserveLabelNop, + virSecurityDomainReleaseLabelNop, + + virSecurityDomainGetProcessLabelNop, + virSecurityDomainSetProcessLabelNop, + + virSecurityDomainSetAllLabelNop, + virSecurityDomainRestoreAllLabelNop, + + virSecurityDomainSetHostdevLabelNop, + virSecurityDomainRestoreHostdevLabelNop, + + virSecurityDomainSetSavedStateLabelNop, + virSecurityDomainRestoreSavedStateLabelNop, +}; diff --git a/src/security/security_nop.h b/src/security/security_nop.h new file mode 100644 index 0000000..714e646 --- /dev/null +++ b/src/security/security_nop.h @@ -0,0 +1,17 @@ +/* + * Copyright (C) 2010 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + */ +#ifndef __VIR_SECURITY_NOP_H__ +# define __VIR_SECURITY_NOP_H__ + +#include "security_driver.h" + +extern virSecurityDriver virSecurityDriverNop; + +#endif /* __VIR_SECURITY_NOP_H__ */ diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 996177a..3f3fe90 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -157,7 +157,7 @@ SELinuxInitialize(void) } static int -SELinuxGenSecurityLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, +SELinuxGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainObjPtr vm) { int rc = -1; @@ -222,7 +222,7 @@ done: } static int -SELinuxReserveSecurityLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, +SELinuxReserveSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainObjPtr vm) { security_context_t pctx; @@ -267,20 +267,34 @@ SELinuxSecurityDriverProbe(void) } static int -SELinuxSecurityDriverOpen(virSecurityDriverPtr drv, - bool allowDiskFormatProbing) +SELinuxSecurityDriverOpen(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + return SELinuxInitialize(); +} + +static int +SELinuxSecurityDriverClose(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + return 0; +} + + +static const char *SELinuxSecurityGetModel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + return SECURITY_SELINUX_NAME; +} + +static const char *SELinuxSecurityGetDOI(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) { /* * Where will the DOI come from? SELinux configuration, or qemu * configuration? For the moment, we'll just set it to "0". */ - virSecurityDriverSetDOI(drv, SECURITY_SELINUX_VOID_DOI); - virSecurityDriverSetAllowDiskFormatProbing(drv, allowDiskFormatProbing); - return SELinuxInitialize(); + return SECURITY_SELINUX_VOID_DOI; } static int -SELinuxGetSecurityProcessLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, +SELinuxGetSecurityProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainObjPtr vm, virSecurityLabelPtr sec) { @@ -393,7 +407,7 @@ err: } static int -SELinuxRestoreSecurityImageLabelInt(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, +SELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainObjPtr vm, virDomainDiskDefPtr disk, int migrated) @@ -438,11 +452,11 @@ SELinuxRestoreSecurityImageLabelInt(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, static int -SELinuxRestoreSecurityImageLabel(virSecurityDriverPtr drv, +SELinuxRestoreSecurityImageLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm, virDomainDiskDefPtr disk) { - return SELinuxRestoreSecurityImageLabelInt(drv, vm, disk, 0); + return SELinuxRestoreSecurityImageLabelInt(mgr, vm, disk, 0); } @@ -476,13 +490,13 @@ SELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk, } static int -SELinuxSetSecurityImageLabel(virSecurityDriverPtr drv, +SELinuxSetSecurityImageLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm, virDomainDiskDefPtr disk) { const virSecurityLabelDefPtr secdef = &vm->def->seclabel; - bool allowDiskFormatProbing = virSecurityDriverGetAllowDiskFormatProbing(drv); + bool allowDiskFormatProbing = virSecurityManagerGetAllowDiskFormatProbing(mgr); if (secdef->type == VIR_DOMAIN_SECLABEL_STATIC) return 0; @@ -516,7 +530,7 @@ SELinuxSetSecurityUSBLabel(usbDevice *dev ATTRIBUTE_UNUSED, } static int -SELinuxSetSecurityHostdevLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, +SELinuxSetSecurityHostdevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainObjPtr vm, virDomainHostdevDefPtr dev) @@ -585,7 +599,7 @@ SELinuxRestoreSecurityUSBLabel(usbDevice *dev ATTRIBUTE_UNUSED, } static int -SELinuxRestoreSecurityHostdevLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, +SELinuxRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainObjPtr vm, virDomainHostdevDefPtr dev) @@ -734,7 +748,7 @@ SELinuxRestoreSecurityChardevCallback(virDomainDefPtr def ATTRIBUTE_UNUSED, static int -SELinuxRestoreSecurityAllLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, +SELinuxRestoreSecurityAllLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainObjPtr vm, int migrated ATTRIBUTE_UNUSED) { @@ -748,13 +762,13 @@ SELinuxRestoreSecurityAllLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, return 0; for (i = 0 ; i < vm->def->nhostdevs ; i++) { - if (SELinuxRestoreSecurityHostdevLabel(drv, + if (SELinuxRestoreSecurityHostdevLabel(mgr, vm, vm->def->hostdevs[i]) < 0) rc = -1; } for (i = 0 ; i < vm->def->ndisks ; i++) { - if (SELinuxRestoreSecurityImageLabelInt(drv, + if (SELinuxRestoreSecurityImageLabelInt(mgr, vm, vm->def->disks[i], migrated) < 0) @@ -779,7 +793,7 @@ SELinuxRestoreSecurityAllLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, } static int -SELinuxReleaseSecurityLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, +SELinuxReleaseSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainObjPtr vm) { const virSecurityLabelDefPtr secdef = &vm->def->seclabel; @@ -803,7 +817,7 @@ SELinuxReleaseSecurityLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, static int -SELinuxSetSavedStateLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, +SELinuxSetSavedStateLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainObjPtr vm, const char *savefile) { @@ -817,7 +831,7 @@ SELinuxSetSavedStateLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, static int -SELinuxRestoreSavedStateLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, +SELinuxRestoreSavedStateLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainObjPtr vm, const char *savefile) { @@ -831,9 +845,19 @@ SELinuxRestoreSavedStateLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, static int -SELinuxSecurityVerify(virDomainDefPtr def) +SELinuxSecurityVerify(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, + virDomainDefPtr def) { const virSecurityLabelDefPtr secdef = &def->seclabel; + if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, + _("security label driver mismatch: " + "'%s' model configured for domain, but " + "hypervisor driver is '%s'."), + secdef->model, virSecurityManagerGetModel(mgr)); + return -1; + } + if (secdef->type == VIR_DOMAIN_SECLABEL_STATIC) { if (security_check_context(secdef->label) != 0) { virSecurityReportError(VIR_ERR_XML_ERROR, @@ -845,7 +869,7 @@ SELinuxSecurityVerify(virDomainDefPtr def) } static int -SELinuxSetSecurityProcessLabel(virSecurityDriverPtr drv, +SELinuxSetSecurityProcessLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm) { /* TODO: verify DOI */ @@ -854,12 +878,12 @@ SELinuxSetSecurityProcessLabel(virSecurityDriverPtr drv, if (vm->def->seclabel.label == NULL) return 0; - if (!STREQ(drv->name, secdef->model)) { + if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " "'%s' model configured for domain, but " "hypervisor driver is '%s'."), - secdef->model, drv->name); + secdef->model, virSecurityManagerGetModel(mgr)); if (security_getenforce() == 1) return -1; } @@ -876,7 +900,7 @@ SELinuxSetSecurityProcessLabel(virSecurityDriverPtr drv, } static int -SELinuxSetSecuritySocketLabel(virSecurityDriverPtr drv, +SELinuxSetSecuritySocketLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm) { /* TODO: verify DOI */ @@ -889,12 +913,12 @@ SELinuxSetSecuritySocketLabel(virSecurityDriverPtr drv, if (vm->def->seclabel.label == NULL) return 0; - if (!STREQ(drv->name, secdef->model)) { + if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " "'%s' model configured for domain, but " "hypervisor driver is '%s'."), - secdef->model, drv->name); + secdef->model, virSecurityManagerGetModel(mgr)); goto done; } @@ -947,7 +971,7 @@ done: } static int -SELinuxClearSecuritySocketLabel(virSecurityDriverPtr drv, +SELinuxClearSecuritySocketLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm) { /* TODO: verify DOI */ @@ -956,12 +980,12 @@ SELinuxClearSecuritySocketLabel(virSecurityDriverPtr drv, if (vm->def->seclabel.label == NULL) return 0; - if (!STREQ(drv->name, secdef->model)) { + if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " "'%s' model configured for domain, but " "hypervisor driver is '%s'."), - secdef->model, drv->name); + secdef->model, virSecurityManagerGetModel(mgr)); if (security_getenforce() == 1) return -1; } @@ -989,7 +1013,7 @@ SELinuxSetSecurityChardevCallback(virDomainDefPtr def ATTRIBUTE_UNUSED, static int -SELinuxSetSecurityAllLabel(virSecurityDriverPtr drv, +SELinuxSetSecurityAllLabel(virSecurityManagerPtr mgr, virDomainObjPtr vm, const char *stdin_path) { @@ -1006,12 +1030,12 @@ SELinuxSetSecurityAllLabel(virSecurityDriverPtr drv, vm->def->disks[i]->src, vm->def->disks[i]->dst); continue; } - if (SELinuxSetSecurityImageLabel(drv, + if (SELinuxSetSecurityImageLabel(mgr, vm, vm->def->disks[i]) < 0) return -1; } for (i = 0 ; i < vm->def->nhostdevs ; i++) { - if (SELinuxSetSecurityHostdevLabel(drv, + if (SELinuxSetSecurityHostdevLabel(mgr, vm, vm->def->hostdevs[i]) < 0) return -1; @@ -1041,24 +1065,37 @@ SELinuxSetSecurityAllLabel(virSecurityDriverPtr drv, return 0; } -virSecurityDriver virSELinuxSecurityDriver = { - .name = SECURITY_SELINUX_NAME, - .probe = SELinuxSecurityDriverProbe, - .open = SELinuxSecurityDriverOpen, - .domainSecurityVerify = SELinuxSecurityVerify, - .domainSetSecurityImageLabel = SELinuxSetSecurityImageLabel, - .domainSetSecuritySocketLabel = SELinuxSetSecuritySocketLabel, - .domainClearSecuritySocketLabel = SELinuxClearSecuritySocketLabel, - .domainRestoreSecurityImageLabel = SELinuxRestoreSecurityImageLabel, - .domainGenSecurityLabel = SELinuxGenSecurityLabel, - .domainReserveSecurityLabel = SELinuxReserveSecurityLabel, - .domainReleaseSecurityLabel = SELinuxReleaseSecurityLabel, - .domainGetSecurityProcessLabel = SELinuxGetSecurityProcessLabel, - .domainSetSecurityProcessLabel = SELinuxSetSecurityProcessLabel, - .domainRestoreSecurityAllLabel = SELinuxRestoreSecurityAllLabel, - .domainSetSecurityAllLabel = SELinuxSetSecurityAllLabel, - .domainSetSecurityHostdevLabel = SELinuxSetSecurityHostdevLabel, - .domainRestoreSecurityHostdevLabel = SELinuxRestoreSecurityHostdevLabel, - .domainSetSavedStateLabel = SELinuxSetSavedStateLabel, - .domainRestoreSavedStateLabel = SELinuxRestoreSavedStateLabel, +virSecurityDriver virSecurityDriverSELinux = { + 0, + SECURITY_SELINUX_NAME, + SELinuxSecurityDriverProbe, + SELinuxSecurityDriverOpen, + SELinuxSecurityDriverClose, + + SELinuxSecurityGetModel, + SELinuxSecurityGetDOI, + + SELinuxSecurityVerify, + + SELinuxSetSecurityImageLabel, + SELinuxRestoreSecurityImageLabel, + + SELinuxSetSecuritySocketLabel, + SELinuxClearSecuritySocketLabel, + + SELinuxGenSecurityLabel, + SELinuxReserveSecurityLabel, + SELinuxReleaseSecurityLabel, + + SELinuxGetSecurityProcessLabel, + SELinuxSetSecurityProcessLabel, + + SELinuxSetSecurityAllLabel, + SELinuxRestoreSecurityAllLabel, + + SELinuxSetSecurityHostdevLabel, + SELinuxRestoreSecurityHostdevLabel, + + SELinuxSetSavedStateLabel, + SELinuxRestoreSavedStateLabel, }; diff --git a/src/security/security_selinux.h b/src/security/security_selinux.h index 056ba75..aa67421 100644 --- a/src/security/security_selinux.h +++ b/src/security/security_selinux.h @@ -13,6 +13,6 @@ #ifndef __VIR_SECURITY_SELINUX_H__ # define __VIR_SECURITY_SELINUX_H__ -extern virSecurityDriver virSELinuxSecurityDriver; +extern virSecurityDriver virSecurityDriverSELinux; #endif /* __VIR_SECURITY_SELINUX_H__ */ diff --git a/src/security/security_stack.c b/src/security/security_stack.c new file mode 100644 index 0000000..9b3753a --- /dev/null +++ b/src/security/security_stack.c @@ -0,0 +1,383 @@ +/* + * Copyright (C) 2010 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * QEMU stacked security driver + */ + +#include <config.h> + +#include "security_stack.h" + +#include "virterror_internal.h" + +#define VIR_FROM_THIS VIR_FROM_SECURITY + +typedef struct _virSecurityStackData virSecurityStackData; +typedef virSecurityStackData *virSecurityStackDataPtr; + +struct _virSecurityStackData { + virSecurityManagerPtr primary; + virSecurityManagerPtr secondary; +}; + +void virSecurityStackSetPrimary(virSecurityManagerPtr mgr, + virSecurityManagerPtr primary) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + priv->primary = primary; +} + +void virSecurityStackSetSecondary(virSecurityManagerPtr mgr, + virSecurityManagerPtr secondary) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + priv->secondary = secondary; +} + +static virSecurityDriverStatus +virSecurityStackProbe(void) +{ + return SECURITY_DRIVER_ENABLE; +} + +static int +virSecurityStackOpen(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int +virSecurityStackClose(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +{ + return 0; +} + +static const char * virSecurityStackGetModel(virSecurityManagerPtr mgr) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + + return virSecurityManagerGetModel(priv->primary); +} + +static const char * virSecurityStackGetDOI(virSecurityManagerPtr mgr) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + + return virSecurityManagerGetDOI(priv->primary); +} + +static int +virSecurityStackVerify(virSecurityManagerPtr mgr, + virDomainDefPtr def) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int rc = 0; + + if (virSecurityManagerVerify(priv->primary, def) < 0) + rc = -1; + +#if 0 + if (virSecurityManagerVerify(priv->secondary, def) < 0) + rc = -1; +#endif + + return rc; +} + + +static int +virSecurityStackGenLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int rc = 0; + + if (virSecurityManagerGenLabel(priv->primary, vm) < 0) + rc = -1; +#if 0 + if (virSecurityManagerGenLabel(priv->secondary, vm) < 0) + rc = -1; +#endif + + return rc; +} + + +static int +virSecurityStackReleaseLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int rc = 0; + + if (virSecurityManagerReleaseLabel(priv->primary, vm) < 0) + rc = -1; +#if 0 + if (virSecurityManagerReleaseLabel(priv->secondary, vm) < 0) + rc = -1; +#endif + + return rc; +} + + +static int +virSecurityStackReserveLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int rc = 0; + + if (virSecurityManagerReserveLabel(priv->primary, vm) < 0) + rc = -1; +#if 0 + if (virSecurityManagerReserveLabel(priv->secondary, vm) < 0) + rc = -1; +#endif + + return rc; +} + + +static int +virSecurityStackSetSecurityImageLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int rc = 0; + + if (virSecurityManagerSetImageLabel(priv->secondary, vm, disk) < 0) + rc = -1; + if (virSecurityManagerSetImageLabel(priv->primary, vm, disk) < 0) + rc = -1; + + return rc; +} + + +static int +virSecurityStackRestoreSecurityImageLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int rc = 0; + + if (virSecurityManagerRestoreImageLabel(priv->secondary, vm, disk) < 0) + rc = -1; + if (virSecurityManagerRestoreImageLabel(priv->primary, vm, disk) < 0) + rc = -1; + + return rc; +} + + +static int +virSecurityStackSetSecurityHostdevLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + virDomainHostdevDefPtr dev) + +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int rc = 0; + + if (virSecurityManagerSetHostdevLabel(priv->secondary, vm, dev) < 0) + rc = -1; + if (virSecurityManagerSetHostdevLabel(priv->primary, vm, dev) < 0) + rc = -1; + + return rc; +} + + +static int +virSecurityStackRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + virDomainHostdevDefPtr dev) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int rc = 0; + + if (virSecurityManagerRestoreHostdevLabel(priv->secondary, vm, dev) < 0) + rc = -1; + if (virSecurityManagerRestoreHostdevLabel(priv->primary, vm, dev) < 0) + rc = -1; + + return rc; +} + + +static int +virSecurityStackSetSecurityAllLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + const char *stdin_path) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int rc = 0; + + if (virSecurityManagerSetAllLabel(priv->secondary, vm, stdin_path) < 0) + rc = -1; + if (virSecurityManagerSetAllLabel(priv->primary, vm, stdin_path) < 0) + rc = -1; + + return rc; +} + + +static int +virSecurityStackRestoreSecurityAllLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + int migrated) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int rc = 0; + + if (virSecurityManagerRestoreAllLabel(priv->secondary, vm, migrated) < 0) + rc = -1; + if (virSecurityManagerRestoreAllLabel(priv->primary, vm, migrated) < 0) + rc = -1; + + return rc; +} + + +static int +virSecurityStackSetSavedStateLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + const char *savefile) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int rc = 0; + + if (virSecurityManagerSetSavedStateLabel(priv->secondary, vm, savefile) < 0) + rc = -1; + if (virSecurityManagerSetSavedStateLabel(priv->primary, vm, savefile) < 0) + rc = -1; + + return rc; +} + + +static int +virSecurityStackRestoreSavedStateLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + const char *savefile) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int rc = 0; + + if (virSecurityManagerRestoreSavedStateLabel(priv->secondary, vm, savefile) < 0) + rc = -1; + if (virSecurityManagerRestoreSavedStateLabel(priv->primary, vm, savefile) < 0) + rc = -1; + + return rc; +} + + +static int +virSecurityStackSetProcessLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int rc = 0; + + if (virSecurityManagerSetProcessLabel(priv->secondary, vm) < 0) + rc = -1; + if (virSecurityManagerSetProcessLabel(priv->primary, vm) < 0) + rc = -1; + + return rc; +} + +static int +virSecurityStackGetProcessLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm, + virSecurityLabelPtr seclabel) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int rc = 0; + +#if 0 + if (virSecurityManagerGetProcessLabel(priv->secondary, vm, seclabel) < 0) + rc = -1; +#endif + if (virSecurityManagerGetProcessLabel(priv->primary, vm, seclabel) < 0) + rc = -1; + + return rc; +} + + +static int +virSecurityStackSetSocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int rc = 0; + + if (virSecurityManagerSetSocketLabel(priv->secondary, vm) < 0) + rc = -1; + if (virSecurityManagerSetSocketLabel(priv->primary, vm) < 0) + rc = -1; + + return rc; +} + + +static int +virSecurityStackClearSocketLabel(virSecurityManagerPtr mgr, + virDomainObjPtr vm) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + int rc = 0; + + if (virSecurityManagerClearSocketLabel(priv->secondary, vm) < 0) + rc = -1; + if (virSecurityManagerClearSocketLabel(priv->primary, vm) < 0) + rc = -1; + + return rc; +} + + +virSecurityDriver virSecurityDriverStack = { + sizeof(virSecurityStackData), + "stack", + virSecurityStackProbe, + virSecurityStackOpen, + virSecurityStackClose, + + virSecurityStackGetModel, + virSecurityStackGetDOI, + + virSecurityStackVerify, + + virSecurityStackSetSecurityImageLabel, + virSecurityStackRestoreSecurityImageLabel, + + virSecurityStackSetSocketLabel, + virSecurityStackClearSocketLabel, + + virSecurityStackGenLabel, + virSecurityStackReserveLabel, + virSecurityStackReleaseLabel, + + virSecurityStackGetProcessLabel, + virSecurityStackSetProcessLabel, + + virSecurityStackSetSecurityAllLabel, + virSecurityStackRestoreSecurityAllLabel, + + virSecurityStackSetSecurityHostdevLabel, + virSecurityStackRestoreSecurityHostdevLabel, + + virSecurityStackSetSavedStateLabel, + virSecurityStackRestoreSavedStateLabel, +}; diff --git a/src/security/security_stack.h b/src/security/security_stack.h new file mode 100644 index 0000000..c924842 --- /dev/null +++ b/src/security/security_stack.h @@ -0,0 +1,24 @@ +/* + * Copyright (C) 2010 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * QEMU stacked security driver + */ + +#include "security_driver.h" + +#ifndef __VIR_SECURITY_STACK +# define __VIR_SECURITY_STACK + +extern virSecurityDriver virSecurityDriverStack; + +void virSecurityStackSetPrimary(virSecurityManagerPtr mgr, + virSecurityManagerPtr primary); +void virSecurityStackSetSecondary(virSecurityManagerPtr mgr, + virSecurityManagerPtr secondary); + +#endif /* __VIR_SECURITY_DAC */ -- 1.7.2.3

Define the basic framework lock manager plugins. The basic plugin API for 3rd parties to implemented is defined in include/libvirt/plugins/lock_manager.h The libvirt code for loading & calling into plugins is in src/locking/lock_manager.{c,h} To allow QEMU driver code to assume that there is always a lock manager present, a simple 'nop' implementation is provided and linked in so that it is guarenteed to be available. This impl returns success for everything, so it should be no functional change vs previous libvirt releases. * configure.ac: Add include/libvirt/plugins/Makefile * include/libvirt/Makefile.am: Add plugins sub directory * include/libvirt/plugins/Makefile.am: Install plugin headers * include/libvirt/plugins/lock_manager.h: Public API for implementing plugins * include/libvirt/virterror.h, src/util/virterror.c: Add VIR_FROM_LOCKING * src/locking/lock_manager.c, src/locking/lock_manager.h: Internal API for managing locking * src/locking/lock_manager_nop.c, src/locking/lock_manager_nop.h: A lock manager that always succeeds * src/locking/lock_manager_nop.h, src/locking/domain_lock.c: Higher level API targetted at locking virDomainObj instances. * src/Makefile.am: Add locking code --- configure.ac | 1 + include/libvirt/Makefile.am | 2 + include/libvirt/plugins/Makefile.am | 3 + include/libvirt/plugins/lock_manager.h | 356 ++++++++++++++++++++++++++++++++ include/libvirt/virterror.h | 1 + po/POTFILES.in | 1 + src/Makefile.am | 5 +- src/libvirt_private.syms | 31 +++ src/locking/README | 158 ++++++++++++++ src/locking/domain_lock.c | 343 ++++++++++++++++++++++++++++++ src/locking/domain_lock.h | 50 +++++ src/locking/lock_manager.c | 348 +++++++++++++++++++++++++++++++ src/locking/lock_manager.h | 77 +++++++ src/locking/lock_manager_nop.c | 159 ++++++++++++++ src/locking/lock_manager_nop.h | 4 + src/util/virterror.c | 3 + 16 files changed, 1541 insertions(+), 1 deletions(-) create mode 100644 include/libvirt/plugins/Makefile.am create mode 100644 include/libvirt/plugins/lock_manager.h create mode 100644 src/locking/README create mode 100644 src/locking/domain_lock.c create mode 100644 src/locking/domain_lock.h create mode 100644 src/locking/lock_manager.c create mode 100644 src/locking/lock_manager.h create mode 100644 src/locking/lock_manager_nop.c create mode 100644 src/locking/lock_manager_nop.h diff --git a/configure.ac b/configure.ac index d21d558..b912a7a 100644 --- a/configure.ac +++ b/configure.ac @@ -2245,6 +2245,7 @@ AC_OUTPUT(Makefile src/Makefile include/Makefile docs/Makefile \ libvirt.pc libvirt.spec mingw32-libvirt.spec \ po/Makefile.in \ include/libvirt/Makefile include/libvirt/libvirt.h \ + include/libvirt/plugins/Makefile \ python/Makefile python/tests/Makefile \ daemon/Makefile \ tools/Makefile \ diff --git a/include/libvirt/Makefile.am b/include/libvirt/Makefile.am index b2c2b76..1795254 100644 --- a/include/libvirt/Makefile.am +++ b/include/libvirt/Makefile.am @@ -6,6 +6,8 @@ virinc_HEADERS = libvirt.h \ libvirt-qemu.h \ virterror.h +SUBDIRS = plugins + install-exec-hook: $(mkinstalldirs) $(DESTDIR)$(virincdir) diff --git a/include/libvirt/plugins/Makefile.am b/include/libvirt/plugins/Makefile.am new file mode 100644 index 0000000..0626d74 --- /dev/null +++ b/include/libvirt/plugins/Makefile.am @@ -0,0 +1,3 @@ + +pluginincdir = $(includedir)/libvirt/plugins +plugininc_HEADERS = lock_manager.h diff --git a/include/libvirt/plugins/lock_manager.h b/include/libvirt/plugins/lock_manager.h new file mode 100644 index 0000000..c22621b --- /dev/null +++ b/include/libvirt/plugins/lock_manager.h @@ -0,0 +1,356 @@ +/* + * lock_manager.h: Defines the lock manager plugin API + * + * Copyright (C) 2010 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#ifndef __VIR_PLUGINS_LOCK_MANAGER_H__ +# define __VIR_PLUGINS_LOCK_MANAGER_H__ + +#include <stdarg.h> + +typedef struct _virLockManager virLockManager; +typedef virLockManager *virLockManagerPtr; + +typedef struct _virLockDriver virLockDriver; +typedef virLockDriver *virLockDriverPtr; + +enum { + /* The managed object is a virtual guest domain */ + VIR_LOCK_MANAGER_OBJECT_TYPE_DOMAIN = 0, +} virLockManagerObjectType; + +/* + * Flags to pass to 'load_drv' and also 'new_drv' method + * Plugins must support at least one of the modes. If a + * mode is unsupported, it must return an error + */ +enum { + VIR_LOCK_MANAGER_MODE_CONTENT = (1 << 0), + VIR_LOCK_MANAGER_MODE_METADATA = (1 << 1), +} virLockManagerFlags; + +enum { + /* The resource to be locked is a virtual disk */ + VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK = 0, +} virLockManagerResourceType; + +typedef enum { + /* The resource is assigned in readonly mode */ + VIR_LOCK_MANAGER_RESOURCE_READONLY = (1 << 0), + /* The resource is assigned in shared, writable mode */ + VIR_LOCK_MANAGER_RESOURCE_SHARED = (1 << 1), +} virLockManagerResourceFlags; + +/* + * Changes in major version denote incompatible ABI changes + * Changes in minor version denote new compatible API entry points + * Changes in micro version denote new compatible flags + */ +# define VIR_LOCK_MANAGER_VERSION_MAJOR 1 +# define VIR_LOCK_MANAGER_VERSION_MINOR 0 +# define VIR_LOCK_MANAGER_VERSION_MICRO 0 + +# define VIR_LOCK_MANAGER_VERSION \ + ((VIR_LOCK_MANAGER_VERSION_MAJOR * 1000 * 1000) + \ + (VIR_LOCK_MANAGER_VERSION_MINOR * 1000) + \ + (VIR_LOCK_MANAGER_VERSION_MICRO)) + + +typedef void (*virLockDriverError)(const char *fmt, ...); +typedef void (*virLockDriverErrorV)(const char *fmt, va_list args); + +/** + * virLockDriverInit: + * @version: the libvirt requested plugin ABI version + * @flags: the libvirt requested plugin optional extras + * + * Allow the plugin to validate the libvirt requested + * plugin version / flags. This allows the plugin impl + * to block its use in versions of libvirtd which are + * too old to support key features. + * + * NB: A plugin may be loaded multiple times, for different + * libvirt drivers (eg QEMU, LXC, UML) + * + * Returns -1 if the requested version/flags were inadequate + */ +typedef int (*virLockDriverInit)(unsigned int version, + unsigned int flags); + +/** + * virLockDriverDeinit: + * + * Called to release any resources prior to the plugin + * being unloaded from memory. Returns -1 to prevent + * plugin from being unloaded from memory. + */ +typedef int (*virLockDriverDeinit)(void); + +/** + * virLockManagerNew: + * @man: the lock manager context + * @type: the type of process to be supervised + * @flags: optional flags, currently unused + * + * Initialize a new context to supervise a process, usually + * a virtual machine. If the lock driver requested a + * private data, the <code>privateData</code> field of + * <code>man</code> will be initialized with a suitable + * pointer. + * + * Returns 0 if successful initialized a new context, -1 on error + */ +typedef int (*virLockDriverNew)(virLockManagerPtr man, + unsigned int type, + unsigned int flags); + +/** + * virLockDriverFree: + * @manager: the lock manager context + * + * Release any resources associated with the lock manager + * context private data + */ +typedef void (*virLockDriverFree)(virLockManagerPtr man); + +/** + * virLockDriverAddResource: + * @manager: the lock manager context + * @type: the resource type virLockManagerResourceType + * @name: the resource name + * @flags: the resource access flags + * + * Assign a resource to a managed object. This will + * only be called prior to the object is being locked + * when it is inactive. eg, to set the initial boot + * time disk assignments on a VM + * The format of @name varies according to + * the resource @type. A VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK + * will have a fully qualified file path. + * + * If no flags are given, the resource is assumed to be + * used in exclusive, read-write mode. Access can be + * relaxed to readonly, or shared read-write. + * + * Returns 0 on success, or -1 on failure + */ +typedef int (*virLockDriverAddResource)(virLockManagerPtr man, + unsigned int type, + const char *name, + unsigned int flags); + +/** + * virLockDriverSetParameter: + * @manager: the lock manager context + * @key: the parameter name + * @value: the parameter value + * + * Set a configuration parameter for the managed process. + * A process of VIR_LOCK_MANAGER_START_DOMAIN will be + * given at least 3 parameters: + * + * - id: the domain unique id + * - uuid: the domain uuid + * - name: the domain name + * + * There may be other parameters specific to the lock manager + * plugin that are provided for the managed process + * + * This should only be called prior to the supervised process + * being started. + * + * Returns 0 on success, or -1 on failure + */ +typedef int (*virLockDriverSetParameter)(virLockManagerPtr man, + const char *key, + const char *value); + +/** + * virLockDriverAcquireObject: + * @manager: the lock manager context + * @state: the current lock state + * @flags: optional flags, currently unused + * + * Start managing resources for the object. If the + * object is being transferred from another location + * the current lock state may be passed in. This + * must be called from the PID that represents the + * object to be managed. If the lock is lost at any + * time, the PID will be killed off by the lock manager. + * + * Returns 0 on success, or -1 on failure + */ +typedef int (*virLockDriverAcquireObject)(virLockManagerPtr man, + const char *state, + unsigned int flags); + +/** + * virLockDriverAttachObject: + * @manager: the lock manager context + * @pid: the managed object PID + * @flags: optional flags, currently unused + * + * Re-attach to an existing lock manager instance managing + * PID @pid. + * + * Returns 0 on success, or -1 on failure + */ +typedef int (*virLockDriverAttachObject)(virLockManagerPtr man, + pid_t pid, + unsigned int flags); + +/** + * virLockDriverDetachObject: + * @manager: the lock manager context + * @pid: the managed object PID + * @flags: optional flags, currently unused + * + * Deattach from an existing lock manager instance managing + * PID @pid. + * + * Returns 0 on success, or -1 on failure + */ +typedef int (*virLockDriverDetachObject)(virLockManagerPtr man, + pid_t pid, + unsigned int flags); + +/** + * virLockDriverReleaseObject: + * @manager: the lock manager context + * @flags: optional flags + * + * Inform the lock manager that the supervised process has + * been, or can be stopped. This can must be called from + * the same context as the previous virLockDriverAttachObject + * or virLockDriverAcquireObject call. + * + * Returns 0 on success, or -1 on failure + */ +typedef int (*virLockDriverReleaseObject)(virLockManagerPtr man, + unsigned int flags); + +/** + * virLockDriverGetState: + * @manager: the lock manager context + * @state: pointer to be filled with lock state + * @flags: optional flags, currently unused + * + * Retrieve the current lock state. The returned + * lock state may be NULL if none is required. The + * caller is responsible for freeing the lock + * state string when it is no longer required + * + * Returns 0 on success, or -1 on failure. + */ +typedef int (*virLockDriverGetState)(virLockManagerPtr man, + char **state, + unsigned int flags); + +/** + * virLockDriverAcquireResource: + * @manager: the lock manager context + * @type: the resource type virLockDriverResourceType + * @name: the resource name + * @flags: the resource access flags + * + * Assign a resource to a managed object. This will + * only be called when the object is already locked + * and active. eg, to hotplug a disk into a VM. + * The format of @name varies according to + * the resource @type. A VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK + * will have a fully qualified file path. + * + * If no flags are given, the resource is assumed to be + * used in exclusive, read-write mode. Access can be + * relaxed to readonly, or shared read-write. + * + * Returns 0 on success, or -1 on failure + */ +typedef int (*virLockDriverAcquireResource)(virLockManagerPtr man, + unsigned int type, + const char *name, + unsigned int flags); + +/** + * virLockDriverReleaseResource: + * @manager: the lock manager context + * @type: the resource type virLockDriverResourceType + * @name: the resource name + * @flags: the resource access flags + * + * Dynamically release a resource for a running process. + * This may only be called after the process has been + * started. The format of @name varies according to + * the resource @type. A VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK + * will have a fully qualified file path. + * + * If no flags are given, the resource is assumed to be + * used in exclusive, read-write mode. Access can be + * relaxed to readonly, or shared read-write. + * + * Returns 0 on success, or -1 on failure + */ +typedef int (*virLockDriverReleaseResource)(virLockManagerPtr man, + unsigned int type, + const char *name, + unsigned int flags); + +struct _virLockManager { + virLockDriverPtr driver; + virLockDriverError error; + virLockDriverErrorV errorV; + void *privateData; +}; + +/** + * The plugin must export a static instance of this + * driver table, with the name 'virLockDriverImpl' + */ +struct _virLockDriver { + /** + * @version: the newest implemented plugin ABI version + * @flags: optional flags, currently unused + */ + unsigned int version; + unsigned int flags; + + size_t privateDataLen; + + virLockDriverInit drvInit; + virLockDriverDeinit drvDeinit; + + virLockDriverNew drvNew; + virLockDriverFree drvFree; + + virLockDriverAddResource drvAddResource; + virLockDriverSetParameter drvSetParameter; + + virLockDriverAcquireObject drvAcquireObject; + virLockDriverAttachObject drvAttachObject; + virLockDriverDetachObject drvDetachObject; + virLockDriverReleaseObject drvReleaseObject; + + virLockDriverGetState drvGetState; + + virLockDriverAcquireResource drvAcquireResource; + virLockDriverAcquireResource drvReleaseResource; +}; + + +#endif /* __VIR_PLUGINS_LOCK_MANAGER_H__ */ diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index eaeb477..c60f7bf 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -76,6 +76,7 @@ typedef enum { VIR_FROM_AUDIT, /* Error from auditing subsystem */ VIR_FROM_SYSINFO, /* Error from sysinfo/SMBIOS */ VIR_FROM_STREAMS, /* Error from I/O streams */ + VIR_FROM_LOCKING, /* Error from lock manager */ } virErrorDomain; diff --git a/po/POTFILES.in b/po/POTFILES.in index 2820ac1..75c259c 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -30,6 +30,7 @@ src/fdstream.c src/interface/netcf_driver.c src/internal.h src/libvirt.c +src/locking/lock_manager.c src/lxc/lxc_container.c src/lxc/lxc_conf.c src/lxc/lxc_controller.c diff --git a/src/Makefile.am b/src/Makefile.am index 66af9cf..750612f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -89,7 +89,10 @@ DRIVER_SOURCES = \ datatypes.c datatypes.h \ fdstream.c fdstream.h \ $(NODE_INFO_SOURCES) \ - libvirt.c libvirt_internal.h + libvirt.c libvirt_internal.h \ + locking/lock_manager.c locking/lock_manager.h \ + locking/lock_manager_nop.c locking/lock_manager_nop.h \ + locking/domain_lock.c locking/domain_lock.h # XML configuration format handling sources diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3af0210..e808375 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -312,6 +312,19 @@ virDomainEventWatchdogNewFromDom; virDomainEventWatchdogNewFromObj; +# domain_lock.h +virDomainLockFree; +virDomainLockForExec; +virDomainLockForStartup; +virDomainLockForShutdown; +virDomainLockForModify; +virDomainLockBeginDiskAttach; +virDomainLockBeginDiskDetach; +virDomainLockEndDiskAttach; +virDomainLockEndDiskDetach; +virDomainLockReleaseAndFree; + + # domain_nwfilter.h virDomainConfNWFilterInstantiate; virDomainConfNWFilterRegister; @@ -479,6 +492,24 @@ virRegisterSecretDriver; virRegisterStorageDriver; +# locking.h +virLockManagerAcquireObject; +virLockManagerAcquireResource; +virLockManagerAddResource; +virLockManagerAttachObject; +virLockManagerDetachObject; +virLockManagerFree; +virLockManagerGetState; +virLockManagerNew; +virLockManagerPluginNew; +virLockManagerPluginRef; +virLockManagerPluginUnref; +virLockManagerReleaseObject; +virLockManagerReleaseResource; +virLockManagerSetParameter; +virLockManagerStartup; + + # logging.h virLogDefineFilter; virLogDefineOutput; diff --git a/src/locking/README b/src/locking/README new file mode 100644 index 0000000..4fa4f89 --- /dev/null +++ b/src/locking/README @@ -0,0 +1,158 @@ + +At libvirtd startup: + + plugin = virLockManagerPluginLoad("sync-manager"); + + +At libvirtd shtudown: + + virLockManagerPluginUnload(plugin) + + +At guest startup: + + manager = virLockManagerNew(plugin, + VIR_LOCK_MANAGER_OBJECT_DOMAIN, + 0); + + virLockManagerSetParameter(manager, "id", id); + virLockManagerSetParameter(manager, "uuid", uuid); + virLockManagerSetParameter(manager, "name", name); + + foreach disk + virLockManagerRegisterResource(manager, + VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK, + disk.path, + ..flags...); + + if (!virLockManagerAcquireObject(manager)) + abort.. + + run QEMU + + +At guest shutdown: + + ...send QEMU 'quit' monitor command, and/or kill(qemupid)... + + if (!virLockManagerShutdown(manager)) + kill(supervisorpid); /* XXX or leave it running ??? */ + + virLockManagerFree(manager); + + + +At libvirtd restart with running guests: + + foreach still running guest + manager = virLockManagerNew(driver, + VIR_LOCK_MANAGER_START_DOMAIN, + VIR_LOCK_MANAGER_NEW_ATTACH); + virLockManagerSetParameter(manager, "id", id); + virLockManagerSetParameter(manager, "uuid", uuid); + virLockManagerSetParameter(manager, "name", name); + + if (!virLockManagerGetChild(manager, &qemupid)) + kill(supervisorpid); /* XXX or leave it running ??? */ + + + +With disk hotplug: + + if (virLockManagerAcquireResource(manager, + VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK, + disk.path + ..flags..)) + ...abort hotplug attempt ... + + ...hotplug the device... + + + +With disk unhotplug: + + ...hotunplug the device... + + if (virLockManagerReleaseResource(manager, + VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK, + disk.path + ..flags..)) + ...log warning ... + + + +During migration: + + 1. On source host + + if (!virLockManagerPrepareMigrate(manager, hosturi)) + ..don't start migration.. + + 2. On dest host + + manager = virLockManagerNew(driver, + VIR_LOCK_MANAGER_START_DOMAIN, + VIR_LOCK_MANAGER_NEW_MIGRATE); + virLockManagerSetParameter(manager, "id", id); + virLockManagerSetParameter(manager, "uuid", uuid); + virLockManagerSetParameter(manager, "name", name); + + foreach disk + virLockManagerRegisterResource(manager, + VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK, + disk.path, + ..flags...); + + char **supervisorargv; + int supervisorargc; + + supervisor = virLockManagerGetSupervisorPath(manager); + virLockManagerGetSupervisorArgs(&argv, &argc); + + cmd = qemuBuildCommandLine(supervisor, supervisorargv, supervisorargv); + + supervisorpid = virCommandExec(cmd); + + if (!virLockManagerGetChild(manager, &qemupid)) + kill(supervisorpid); /* XXX or leave it running ??? */ + + 3. Initiate migration in QEMU on source and wait for completion + + 4a. On failure + + 4a1 On target + + virLockManagerCompleteMigrateIn(manager, + VIR_LOCK_MANAGER_MIGRATE_CANCEL); + virLockManagerShutdown(manager); + virLockManagerFree(manager); + + 4a2 On source + + virLockManagerCompleteMigrateIn(manager, + VIR_LOCK_MANAGER_MIGRATE_CANCEL); + + 4b. On succcess + + + 4b1 On target + + virLockManagerCompleteMigrateIn(manager, 0); + + 42 On source + + virLockManagerCompleteMigrateIn(manager, 0); + virLockManagerShutdown(manager); + virLockManagerFree(manager); + + +Notes: + + - If a lock manager impl does just VM level leases, it can + ignore all the resource paths at startup. + + - If a lock manager impl does not support migrate + it can return an error from all migrate calls + + - If a lock manger impl does not support hotplug + it can return an error from all resource acquire/release calls diff --git a/src/locking/domain_lock.c b/src/locking/domain_lock.c new file mode 100644 index 0000000..086ab4d --- /dev/null +++ b/src/locking/domain_lock.c @@ -0,0 +1,343 @@ + +#include <config.h> + +#include "domain_lock.h" +#include "memory.h" +#include "uuid.h" +#include "virterror_internal.h" + +#define VIR_FROM_THIS VIR_FROM_LOCKING + +struct _virDomainLock { + virLockManagerPtr contentLock; + virLockManagerPtr metadataLock; + + pid_t pid; + + bool releaseContentLock; + bool releaseMetadataLock; +}; + +static virLockManagerPtr virDomainLockNewManager(virLockManagerPluginPtr plugin, + virDomainObjPtr vm, + const char *state, + int mode, + bool acquireLock) +{ + virLockManagerPtr lock; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + int i; + + if (!(lock = virLockManagerNew(plugin, + mode, + VIR_LOCK_MANAGER_OBJECT_TYPE_DOMAIN))) + return NULL; + + virUUIDFormat(vm->def->uuid, uuidstr); + if (virLockManagerSetParameter(lock, + "uuid", uuidstr) < 0) + goto error; + if (virLockManagerSetParameter(lock, + "name", vm->def->name) < 0) + goto error; + + if (acquireLock) { + for (i = 0 ; i < vm->def->ndisks ; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + unsigned int diskFlags = 0; + if (!disk->src) + continue; + + if (!(disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK || + disk->type == VIR_DOMAIN_DISK_TYPE_FILE || + disk->type == VIR_DOMAIN_DISK_TYPE_DIR)) + continue; + + if (disk->readonly) + diskFlags |= VIR_LOCK_MANAGER_RESOURCE_READONLY; + if (disk->shared) + diskFlags |= VIR_LOCK_MANAGER_RESOURCE_SHARED; + + if (virLockManagerAddResource(lock, + VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK, + disk->src, + diskFlags) < 0) { + virLockManagerFree(lock); + return NULL; + } + } + + if (virLockManagerAcquireObject(lock, state, 0) < 0) + goto error; + } else { + if (virLockManagerAttachObject(lock, vm->pid, 0) < 0) + goto error; + } + + return lock; + +error: + virLockManagerFree(lock); + return NULL; +} + + +static virDomainLockPtr virDomainLockNew(virLockManagerPluginPtr contentLockPlugin, + virLockManagerPluginPtr metadataLockPlugin, + virDomainObjPtr dom, + const char *contentState, + const char *metadataState, + bool acquireContentLock, + bool acquireMetadataLock) +{ + virDomainLockPtr lock; + + if (VIR_ALLOC(lock) < 0) { + virReportOOMError(); + goto error; + } + + if (contentLockPlugin && + !(lock->contentLock = virDomainLockNewManager(contentLockPlugin, + dom, + contentState, + VIR_LOCK_MANAGER_MODE_CONTENT, + acquireContentLock))) + goto error; + + lock->releaseContentLock = acquireContentLock; + + if (metadataLockPlugin && + !(lock->metadataLock = virDomainLockNewManager(metadataLockPlugin, + dom, + metadataState, + VIR_LOCK_MANAGER_MODE_METADATA, + acquireMetadataLock))) + goto error; + + lock->releaseMetadataLock = acquireMetadataLock; + + lock->pid = dom->pid; + + return lock; + +error: + virDomainLockFree(lock); + return NULL; +} + + +virDomainLockPtr virDomainLockForExec(virLockManagerPluginPtr contentLockPlugin, + const char *contentState, + virDomainObjPtr dom) +{ + return virDomainLockNew(contentLockPlugin, + NULL, + dom, + contentState, + NULL, + true, + false); +} + +virDomainLockPtr virDomainLockForStartup(virLockManagerPluginPtr contentLockPlugin, + virLockManagerPluginPtr metadataLockPlugin, + const char *metadataState, + virDomainObjPtr dom) +{ + return virDomainLockNew(contentLockPlugin, + metadataLockPlugin, + dom, + NULL, + metadataState, + false, + true); +} + +virDomainLockPtr virDomainLockForShutdown(virLockManagerPluginPtr contentLockPlugin, + virLockManagerPluginPtr metadataLockPlugin, + virDomainObjPtr dom) +{ + return virDomainLockNew(contentLockPlugin, + metadataLockPlugin, + dom, + NULL, + NULL, + true, + true); +} + +virDomainLockPtr virDomainLockForModify(virLockManagerPluginPtr contentLockPlugin, + virLockManagerPluginPtr metadataLockPlugin, + virDomainObjPtr dom) +{ + return virDomainLockNew(contentLockPlugin, + metadataLockPlugin, + dom, + NULL, + NULL, + false, + false); +} + + +static int virDomainLockDiskOperation(virDomainLockPtr lock, + virDomainDiskDefPtr disk, + bool isBegin, + bool isAttach) +{ + unsigned int diskFlags = 0; + if (!disk->src) + return 0; + + if (!(disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK || + disk->type == VIR_DOMAIN_DISK_TYPE_FILE || + disk->type == VIR_DOMAIN_DISK_TYPE_DIR)) + return 0; + + if (disk->readonly) + diskFlags |= VIR_LOCK_MANAGER_RESOURCE_READONLY; + if (disk->shared) + diskFlags |= VIR_LOCK_MANAGER_RESOURCE_SHARED; + + if (isAttach) { + if (isBegin) { + if (lock->contentLock && + virLockManagerAcquireResource(lock->contentLock, + VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK, + disk->src, + diskFlags) < 0) + return -1; + + if (lock->metadataLock && + virLockManagerAcquireResource(lock->metadataLock, + VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK, + disk->src, + diskFlags) < 0) { + virLockManagerReleaseResource(lock->contentLock, + VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK, + disk->src, + diskFlags); + return -1; + } + } else { + if (lock->metadataLock && + virLockManagerReleaseResource(lock->metadataLock, + VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK, + disk->src, + diskFlags) < 0) + return -1; + } + } else { + if (isBegin) { + if (lock->metadataLock && + virLockManagerAcquireResource(lock->metadataLock, + VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK, + disk->src, + diskFlags) < 0) { + return -1; + } + } else { + if (lock->metadataLock && + virLockManagerReleaseResource(lock->metadataLock, + VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK, + disk->src, + diskFlags) < 0) + return -1; + + if (lock->contentLock && + virLockManagerReleaseResource(lock->contentLock, + VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK, + disk->src, + diskFlags) < 0) + return -1; + + } + } + + return 0; +} + +int virDomainLockBeginDiskAttach(virDomainLockPtr lock, + virDomainDiskDefPtr disk) +{ + return virDomainLockDiskOperation(lock, disk, true, true); +} + +int virDomainLockEndDiskAttach(virDomainLockPtr lock, + virDomainDiskDefPtr disk) +{ + return virDomainLockDiskOperation(lock, disk, false, true); +} + + +int virDomainLockBeginDiskDetach(virDomainLockPtr lock, + virDomainDiskDefPtr disk) +{ + return virDomainLockDiskOperation(lock, disk, true, false); +} + + +int virDomainLockEndDiskDetach(virDomainLockPtr lock, + virDomainDiskDefPtr disk) +{ + return virDomainLockDiskOperation(lock, disk, false, false); +} + + +void virDomainLockFree(virDomainLockPtr lock) +{ + if (!lock) + return; + + virLockManagerFree(lock->metadataLock); + virLockManagerFree(lock->contentLock); + + VIR_FREE(lock); +} + +void virDomainLockReleaseAndFree(virDomainLockPtr lock) +{ + if (!lock) + return; + + if (lock->metadataLock) { + if (lock->releaseMetadataLock) + virLockManagerReleaseObject(lock->metadataLock, 0); + else + virLockManagerDetachObject(lock->metadataLock, lock->pid, 0); + } + + if (lock->contentLock) { + if (lock->releaseContentLock) + virLockManagerReleaseObject(lock->contentLock, 0); + else + virLockManagerDetachObject(lock->contentLock, lock->pid, 0); + } + + virDomainLockFree(lock); +} + +int virDomainLockGetState(virDomainLockPtr lock, + char **contentState, + char **metadataState, + unsigned int flags) +{ + *contentState = NULL; + *metadataState = NULL; + + if (lock->contentLock) { + if (virLockManagerGetState(lock->contentLock, contentState, flags) < 0) + return -1; + } + + if (lock->metadataLock) { + if (virLockManagerGetState(lock->metadataLock, metadataState, flags) < 0) { + VIR_FREE(*contentState); + return -1; + } + } + + return 0; +} + diff --git a/src/locking/domain_lock.h b/src/locking/domain_lock.h new file mode 100644 index 0000000..923410e --- /dev/null +++ b/src/locking/domain_lock.h @@ -0,0 +1,50 @@ + +#include "internal.h" +#include "domain_conf.h" +#include "lock_manager.h" + +#ifndef __VIR_DOMAIN_LOCK_H__ +# define __VIR_DOMAIN_LOCK_H__ + +typedef struct _virDomainLock virDomainLock; +typedef virDomainLock *virDomainLockPtr; + +virDomainLockPtr virDomainLockForExec(virLockManagerPluginPtr contentLockPlugin, + const char *state, + virDomainObjPtr dom); + +virDomainLockPtr virDomainLockForStartup(virLockManagerPluginPtr contentLockPlugin, + virLockManagerPluginPtr metadataLockPlugin, + const char *state, + virDomainObjPtr dom); + +virDomainLockPtr virDomainLockForShutdown(virLockManagerPluginPtr contentLockPlugin, + virLockManagerPluginPtr metadataLockPlugin, + virDomainObjPtr dom); + +virDomainLockPtr virDomainLockForModify(virLockManagerPluginPtr contentLockPlugin, + virLockManagerPluginPtr metadataLockPlugin, + virDomainObjPtr dom); + +int virDomainLockBeginDiskAttach(virDomainLockPtr dom, + virDomainDiskDefPtr disk); + +int virDomainLockEndDiskAttach(virDomainLockPtr dom, + virDomainDiskDefPtr disk); + +int virDomainLockBeginDiskDetach(virDomainLockPtr dom, + virDomainDiskDefPtr disk); + +int virDomainLockEndDiskDetach(virDomainLockPtr dom, + virDomainDiskDefPtr disk); + +void virDomainLockFree(virDomainLockPtr lock); +void virDomainLockReleaseAndFree(virDomainLockPtr lock); + +int virDomainLockGetState(virDomainLockPtr manager, + char **contentState, + char **metadataState, + unsigned int flags); + + +#endif /* __VIR_DOMAIN_LOCK_H__ */ diff --git a/src/locking/lock_manager.c b/src/locking/lock_manager.c new file mode 100644 index 0000000..dc5e7d8 --- /dev/null +++ b/src/locking/lock_manager.c @@ -0,0 +1,348 @@ +/* + * lock_manager.c: Implements the internal lock manager API + * + * Copyright (C) 2010 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#include <config.h> + +#include "lock_manager.h" +#include "lock_manager_nop.h" +#include "virterror_internal.h" +#include "logging.h" +#include "util.h" +#include "memory.h" + +#include <dlfcn.h> +#include <stdlib.h> +#include <unistd.h> + +#define VIR_FROM_THIS VIR_FROM_LOCKING + +#define virLockError(code, ...) \ + virReportErrorHelper(NULL, VIR_FROM_THIS, code, __FILE__, \ + __FUNCTION__, __LINE__, __VA_ARGS__) + +#define CHECK_PLUGIN(field, errret) \ + if (!plugin->driver->field) { \ + virLockError(VIR_ERR_INTERNAL_ERROR, \ + _("Missing '%s' field in lock manager driver"), \ + #field); \ + return errret; \ + } + +#define CHECK_MANAGER(field, errret) \ + if (!manager->driver->field) { \ + virLockError(VIR_ERR_INTERNAL_ERROR, \ + _("Missing '%s' field in lock manager driver"), \ + #field); \ + return errret; \ + } + +struct _virLockManagerPlugin { + virLockDriverPtr driver; + void *handle; + int refs; +}; + +#define DEFAULT_LOCK_MANAGER_PLUGIN_DIR LIBDIR "/libvirt/lock_manager" + +static void virLockManagerErrorV(const char *fmt, va_list args ATTRIBUTE_UNUSED) +{ + /* XXX how to pass va-list to virReportErrorFull ??? */ + virLockError(VIR_ERR_INTERNAL_ERROR, "%s", fmt); +} + + +static void virLockManagerError(const char *fmt, ...) +{ + va_list args; + + va_start(args, fmt); + virLockManagerErrorV(fmt, args); + va_end(args); +} + +/** + * virLockManagerPluginNew: + * @name: the name of the plugin + * @flag: optional plugin flags + * + * Attempt to load the plugin $(libdir)/libvirt/lock_manager/(a)name.so + * The plugin driver entry point will be resolved & invoked to obtain + * the lock manager driver. + * + * Even if the loading of the plugin succeeded, this may still + * return NULL if the plugin impl decided that we (libvirtd) + * are too old to support a feature it requires + * + * Returns a plugin object, or NULL if loading failed. + */ +virLockManagerPluginPtr virLockManagerPluginNew(const char *name, + unsigned int flags) +{ + void *handle = NULL; + virLockDriverPtr driver; + virLockManagerPluginPtr plugin; + const char *moddir = getenv("LIBVIRT_LOCK_MANAGER_PLUGIN_DIR"); + char *modfile = NULL; + + if (STREQ(name, "nop")) { + driver = &virLockDriverNop; + } else { + if (moddir == NULL) + moddir = DEFAULT_LOCK_MANAGER_PLUGIN_DIR; + + VIR_DEBUG("Module load %s from %s", name, moddir); + + if (virAsprintf(&modfile, "%s/%s.so", moddir, name) < 0) { + virReportOOMError(); + return NULL; + } + + if (access(modfile, R_OK) < 0) { + virReportSystemError(errno, + _("Plugin %s not accessible"), + modfile); + goto cleanup; + } + + handle = dlopen(modfile, RTLD_NOW | RTLD_LOCAL); + if (!handle) { + virLockError(VIR_ERR_SYSTEM_ERROR, + _("Failed to load plugin %s: %s"), + modfile, dlerror()); + goto cleanup; + } + + if (!(driver = dlsym(handle, "virLockDriverImpl"))) { + virLockError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing plugin initialization symbol 'virLockDriverImpl'")); + goto cleanup; + } + } + + if (driver->drvInit(VIR_LOCK_MANAGER_VERSION, flags) < 0) { + virLockError(VIR_ERR_INTERNAL_ERROR, "%s", + _("plugin ABI is not compatible")); + goto cleanup; + } + + if (VIR_ALLOC(plugin) < 0) { + virReportOOMError(); + goto cleanup; + } + + plugin->driver = driver; + plugin->handle = handle; + plugin->refs = 1; + + VIR_FREE(modfile); + return plugin; + +cleanup: + VIR_FREE(modfile); + if (handle) + dlclose(handle); + return NULL; +} + + +/** + * virLockManagerPluginRef: + * @plugin: the plugin implementation to ref + * + * Acquires an additional reference on the plugin. + */ +void virLockManagerPluginRef(virLockManagerPluginPtr plugin) +{ + plugin->refs++; +} + + +/** + * virLockManagerPluginUnref: + * @plugin: the plugin implementation to unref + * + * Releases a reference on the plugin. When the last reference + * is released, it will attempt to unload the plugin from memory. + * The plugin may refuse to allow unloading if this would + * result in an unsafe scenario. + * + */ +void virLockManagerPluginUnref(virLockManagerPluginPtr plugin) +{ + if (!plugin) + return; + + plugin->refs--; + + if (plugin->refs > 0) + return; + + if (plugin->driver->drvDeinit() >= 0) { + if (plugin->handle) + dlclose(plugin->handle); + } else { + VIR_WARN0("Unable to unload lock maanger plugin from memory"); + return; + } + + VIR_FREE(plugin); +} + + +/** + * virLockManagerNew: + * @plugin: the plugin implementation to use + * @type: the type of process to be supervised + * @flags: optional flags, currently unused + * + * Create a new context to supervise a process, usually + * a virtual machine. + * + * Returns a new lock manager context + */ +virLockManagerPtr virLockManagerNew(virLockManagerPluginPtr plugin, + unsigned int type, + unsigned int flags) +{ + virLockManagerPtr lock; + CHECK_PLUGIN(drvNew, NULL); + + if (VIR_ALLOC_VAR(lock, char, plugin->driver->privateDataLen) < 0) { + virReportOOMError(); + return NULL; + } + + lock->driver = plugin->driver; + lock->error = virLockManagerError; + lock->errorV = virLockManagerErrorV; + lock->privateData = plugin->driver->privateDataLen ? lock + sizeof(*lock) : NULL; + + if (plugin->driver->drvNew(lock, type, flags) < 0) { + VIR_FREE(lock); + return NULL; + } + + return lock; +} + + +int virLockManagerAddResource(virLockManagerPtr manager, + unsigned int type, + const char *name, + unsigned int flags) +{ + CHECK_MANAGER(drvAddResource, -1); + + return manager->driver->drvAddResource(manager, type, name, flags); +} + +int virLockManagerAcquireObject(virLockManagerPtr manager, + const char *state, + unsigned int flags) +{ + CHECK_MANAGER(drvAcquireObject, -1); + + return manager->driver->drvAcquireObject(manager, state, flags); +} + + +int virLockManagerAttachObject(virLockManagerPtr manager, + pid_t pid, + unsigned int flags) +{ + CHECK_MANAGER(drvAttachObject, -1); + + return manager->driver->drvAttachObject(manager, pid, flags); +} + + +int virLockManagerDetachObject(virLockManagerPtr manager, + pid_t pid, + unsigned int flags) +{ + CHECK_MANAGER(drvDetachObject, -1); + + return manager->driver->drvDetachObject(manager, pid, flags); +} + + +int virLockManagerReleaseObject(virLockManagerPtr manager, + unsigned int flags) +{ + CHECK_MANAGER(drvReleaseObject, -1); + + return manager->driver->drvReleaseObject(manager, flags); +} + + +int virLockManagerGetState(virLockManagerPtr manager, + char **state, + unsigned int flags) +{ + CHECK_MANAGER(drvGetState, -1); + + return manager->driver->drvGetState(manager, state, flags); +} + + +int virLockManagerSetParameter(virLockManagerPtr manager, + const char *key, + const char *value) +{ + CHECK_MANAGER(drvSetParameter, -1); + + return manager->driver->drvSetParameter(manager, key, value); +} + + +int virLockManagerAcquireResource(virLockManagerPtr manager, + unsigned int type, + const char *name, + unsigned int flags) +{ + CHECK_MANAGER(drvAcquireResource, -1); + + return manager->driver->drvAcquireResource(manager, type, name, flags); +} + + +int virLockManagerReleaseResource(virLockManagerPtr manager, + unsigned int type, + const char *name, + unsigned int flags) +{ + CHECK_MANAGER(drvReleaseResource, -1); + + return manager->driver->drvReleaseResource(manager, type, name, flags); +} + + +int virLockManagerFree(virLockManagerPtr manager) +{ + if (!manager) + return 0; + + CHECK_MANAGER(drvFree, -1); + + manager->driver->drvFree(manager); + + return 0; +} diff --git a/src/locking/lock_manager.h b/src/locking/lock_manager.h new file mode 100644 index 0000000..35d4433 --- /dev/null +++ b/src/locking/lock_manager.h @@ -0,0 +1,77 @@ +/* + * lock_manager.h: Defines the internal lock manager API + * + * Copyright (C) 2010 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#ifndef __VIR_LOCK_MANAGER_H__ +# define __VIR_LOCK_MANAGER_H__ + +# include "internal.h" +# include <libvirt/plugins/lock_manager.h> + +typedef struct _virLockManagerPlugin virLockManagerPlugin; +typedef virLockManagerPlugin *virLockManagerPluginPtr; + +virLockManagerPluginPtr virLockManagerPluginNew(const char *name, + unsigned int flags); +void virLockManagerPluginRef(virLockManagerPluginPtr plugin); +void virLockManagerPluginUnref(virLockManagerPluginPtr plugin); + +virLockManagerPtr virLockManagerNew(virLockManagerPluginPtr plugin, + unsigned int type, + unsigned int flags); + +int virLockManagerSetParameter(virLockManagerPtr manager, + const char *key, + const char *value); + +int virLockManagerAddResource(virLockManagerPtr manager, + unsigned int type, + const char *name, + unsigned int flags); + +int virLockManagerAcquireObject(virLockManagerPtr manager, + const char *state, + unsigned int flags); +int virLockManagerAttachObject(virLockManagerPtr manager, + pid_t pid, + unsigned int flags); +int virLockManagerDetachObject(virLockManagerPtr manager, + pid_t pid, + unsigned int flags); +int virLockManagerReleaseObject(virLockManagerPtr manager, + unsigned int flags); + +int virLockManagerGetState(virLockManagerPtr manager, + char **state, + unsigned int flags); + +int virLockManagerAcquireResource(virLockManagerPtr manager, + unsigned int type, + const char *name, + unsigned int flags); + +int virLockManagerReleaseResource(virLockManagerPtr manager, + unsigned int type, + const char *name, + unsigned int flags); + +int virLockManagerFree(virLockManagerPtr manager); + +#endif /* __VIR_LOCK_MANAGER_H__ */ diff --git a/src/locking/lock_manager_nop.c b/src/locking/lock_manager_nop.c new file mode 100644 index 0000000..e03d6ec --- /dev/null +++ b/src/locking/lock_manager_nop.c @@ -0,0 +1,159 @@ + +#include <config.h> + +#include "lock_manager_nop.h" +#include "memory.h" +#include "logging.h" + +/** + * + * NB: This file must *not* use regular virReportError functions + * but instead use the lock manager plugin error reporting hooks + * + */ + +static int virLockManagerNopInit(unsigned int version, + unsigned int flags) +{ + VIR_DEBUG("version=%u flags=%u", version, flags); + + return 0; +} + +static int virLockManagerNopDeinit(void) +{ + VIR_DEBUG0(""); + + return 0; +} + +static int virLockManagerNopNew(virLockManagerPtr lock, + unsigned int type, + unsigned int flags) +{ + VIR_DEBUG("lock=%p, type=%u flags=%u", lock, type, flags); + + return 0; +} + +static int virLockManagerNopAddResource(virLockManagerPtr lock, + unsigned int type, + const char *name, + unsigned int flags) +{ + VIR_DEBUG("lock=%p type=%u name=%s flags=%u", lock, type, name, flags); + + return 0; +} + +static int virLockManagerNopSetParameter(virLockManagerPtr lock, + const char *key, + const char *value) +{ + VIR_DEBUG("lock=%p key=%s value=%s", lock, key, value); + + return 0; +} + +static int virLockManagerNopAcquireObject(virLockManagerPtr lock, + const char *state, + unsigned int flags) +{ + VIR_DEBUG("lock=%p state=%s flags=%u", lock, state, flags); + + return 0; +} + +static int virLockManagerNopAttachObject(virLockManagerPtr lock, + pid_t pid, + unsigned int flags) +{ + VIR_DEBUG("lock=%p pid=%u flags=%u", lock, (unsigned int)pid, flags); + + return 0; +} + +static int virLockManagerNopDetachObject(virLockManagerPtr lock, + pid_t pid, + unsigned int flags) +{ + VIR_DEBUG("lock=%p pid=%u flags=%u", lock, (unsigned int)pid, flags); + + return 0; +} + + +static int virLockManagerNopReleaseObject(virLockManagerPtr lock, + unsigned int flags) +{ + VIR_DEBUG("lock=%p flags=%u", lock, flags); + + return 0; +} + +static int virLockManagerNopGetState(virLockManagerPtr lock, + char **state, + unsigned int flags) +{ + VIR_DEBUG("lock=%p state=%p flags=%u", lock, state, flags); + + *state = NULL; + + return 0; +} + + +static int virLockManagerNopAcquireResource(virLockManagerPtr lock, + unsigned int type, + const char *name, + unsigned int flags) +{ + VIR_DEBUG("lock=%p type=%u name=%s flags=%u", lock, type, name, flags); + + return 0; +} + +static int virLockManagerNopReleaseResource(virLockManagerPtr lock, + unsigned int type, + const char *name, + unsigned int flags) +{ + VIR_DEBUG("lock=%p type=%u name=%s flags=%u", lock, type, name, flags); + + return 0; +} + +static void virLockManagerNopFree(virLockManagerPtr man) +{ + VIR_DEBUG("lock=%p", man); + + VIR_FREE(man); +} + +virLockDriver virLockDriverNop = +{ + .version = VIR_LOCK_MANAGER_VERSION, + .flags = VIR_LOCK_MANAGER_MODE_CONTENT | VIR_LOCK_MANAGER_MODE_METADATA, + + .privateDataLen = 0, + + .drvInit = virLockManagerNopInit, + .drvDeinit = virLockManagerNopDeinit, + + .drvNew = virLockManagerNopNew, + .drvFree = virLockManagerNopFree, + + .drvAddResource = virLockManagerNopAddResource, + .drvSetParameter = virLockManagerNopSetParameter, + + .drvAcquireObject = virLockManagerNopAcquireObject, + .drvAttachObject = virLockManagerNopAttachObject, + .drvDetachObject = virLockManagerNopDetachObject, + .drvReleaseObject = virLockManagerNopReleaseObject, + + .drvGetState = virLockManagerNopGetState, + + .drvAcquireResource = virLockManagerNopAcquireResource, + .drvReleaseResource = virLockManagerNopReleaseResource, +}; + diff --git a/src/locking/lock_manager_nop.h b/src/locking/lock_manager_nop.h new file mode 100644 index 0000000..61d6d2c --- /dev/null +++ b/src/locking/lock_manager_nop.h @@ -0,0 +1,4 @@ + +#include "lock_manager.h" + +extern virLockDriver virLockDriverNop; diff --git a/src/util/virterror.c b/src/util/virterror.c index 9757fd4..d524d04 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -196,6 +196,9 @@ static const char *virErrorDomainName(virErrorDomain domain) { case VIR_FROM_STREAMS: dom = "Streams "; break; + case VIR_FROM_LOCKING: + dom = "Locking "; + break; } return(dom); } -- 1.7.2.3

The QEMU integrates with the lock manager instructure in a number of key places * During startup, a lock is acquired in between the fork & exec * During startup, the libvirtd process acquires a lock before setting file labelling * During shutdown, the libvirtd process acquires a lock before restoring file labelling * During hotplug, unplug & media change the libvirtd process holds a lock while setting/restoring labels The main content lock is only ever held by the QEMU child process, or libvirtd during VM shutdown. The rest of the operations only require libvirtd to hold the metadata locks, relying on the active QEMU still holding the content lock. * src/qemu/qemu_conf.c, src/qemu/qemu_conf.h, src/qemu/libvirtd_qemu.aug, src/qemu/test_libvirtd_qemu.aug: Add config parameter for configuring lock managers * src/qemu/qemu_driver.c: Add calls to the lock manager --- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu_conf.c | 30 +++++ src/qemu/qemu_conf.h | 5 + src/qemu/qemu_driver.c | 253 +++++++++++++++++++++++++++++++++------ src/qemu/test_libvirtd_qemu.aug | 4 + 5 files changed, 256 insertions(+), 37 deletions(-) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index 78852f3..5adc447 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -43,6 +43,7 @@ module Libvirtd_qemu = | bool_entry "clear_emulator_capabilities" | bool_entry "allow_disk_format_probing" | bool_entry "set_process_name" + | str_entry "lock_manager" (* Each enty in the config is one of the following three ... *) let entry = vnc_entry diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 35caccc..dce9a63 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -148,6 +148,14 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, */ if (access (filename, R_OK) == -1) { VIR_INFO("Could not read qemu config file %s", filename); + if (!(driver->contentLockManager = + virLockManagerPluginNew("nop", + VIR_LOCK_MANAGER_MODE_CONTENT))) + VIR_ERROR("Failed to load lock manager %s", "nop"); + if (!(driver->metadataLockManager = + virLockManagerPluginNew("nop", + VIR_LOCK_MANAGER_MODE_METADATA))) + VIR_ERROR("Failed to load lock manager %s", "nop"); return 0; } @@ -435,6 +443,28 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, CHECK_TYPE ("set_process_name", VIR_CONF_LONG); if (p) driver->setProcessName = p->l; + p = virConfGetValue (conf, "lock_manager"); + CHECK_TYPE ("lock_manager", VIR_CONF_STRING); + if (p && p->str) { + if (!(driver->contentLockManager = + virLockManagerPluginNew(p->str, + VIR_LOCK_MANAGER_MODE_CONTENT))) + VIR_ERROR("Failed to load lock manager %s", p->str); + if (!(driver->metadataLockManager = + virLockManagerPluginNew(p->str, + VIR_LOCK_MANAGER_MODE_METADATA))) + VIR_ERROR("Failed to load lock manager %s", p->str); + } else { + if (!(driver->contentLockManager = + virLockManagerPluginNew("nop", + VIR_LOCK_MANAGER_MODE_CONTENT))) + VIR_ERROR("Failed to load lock manager %s", "nop"); + if (!(driver->metadataLockManager = + virLockManagerPluginNew("nop", + VIR_LOCK_MANAGER_MODE_METADATA))) + VIR_ERROR("Failed to load lock manager %s", "nop"); + } + virConfFree (conf); return 0; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index ba3c7b1..3789ed2 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -40,6 +40,7 @@ # include "cpu_conf.h" # include "driver.h" # include "bitmap.h" +# include "locking/lock_manager.h" # define qemudDebug(fmt, ...) do {} while(0) @@ -176,6 +177,10 @@ struct qemud_driver { virBitmapPtr reservedVNCPorts; virSysinfoDefPtr hostsysinfo; + + /* These two might point to the same instance */ + virLockManagerPluginPtr contentLockManager; + virLockManagerPluginPtr metadataLockManager; }; typedef struct _qemuDomainPCIAddressSet qemuDomainPCIAddressSet; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2b901b8..9dcf02d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -83,6 +83,8 @@ #include "files.h" #include "fdstream.h" #include "configmake.h" +#include "locking/domain_lock.h" + #define VIR_FROM_THIS VIR_FROM_QEMU @@ -1857,6 +1859,15 @@ qemudStartup(int privileged) { goto error; } + /* We should always at least have the 'nop' manager, so + * NULLs here are a fatal error + */ + if (!qemu_driver->contentLockManager || + !qemu_driver->metadataLockManager) { + VIR_ERROR0("Missing content/metadata lock managers"); + goto error; + } + if (qemudSecurityInit(qemu_driver) < 0) goto error; @@ -2088,6 +2099,9 @@ qemudShutdown(void) { virCgroupFree(&qemu_driver->cgroup); + virLockManagerPluginUnref(qemu_driver->contentLockManager); + virLockManagerPluginUnref(qemu_driver->metadataLockManager); + qemuDriverUnlock(qemu_driver); virMutexDestroy(&qemu_driver->lock); VIR_FREE(qemu_driver); @@ -3650,26 +3664,45 @@ struct qemudHookData { virConnectPtr conn; virDomainObjPtr vm; struct qemud_driver *driver; + char *lockState; }; static int qemudSecurityHook(void *data) { struct qemudHookData *h = data; + virDomainLockPtr lock; + int ret = -1; + + if (!(lock = virDomainLockForExec(h->driver->contentLockManager, + h->lockState, + h->vm))) + goto cleanup; /* This must take place before exec(), so that all QEMU * memory allocation is on the correct NUMA node */ if (qemuAddToCgroup(h->driver, h->vm->def) < 0) - return -1; + goto cleanup; /* This must be done after cgroup placement to avoid resetting CPU * affinity */ if (qemudInitCpuAffinity(h->vm) < 0) - return -1; + goto cleanup; if (virSecurityManagerSetProcessLabel(h->driver->securityManager, h->vm) < 0) - return -1; + goto cleanup; - return 0; + /* XXX temp hack to let disk labelling complete */ + sleep(10); + + ret = 0; + +cleanup: + /* Free, but don't release the lock. The object lock + * must remain for lifetime of QEMU process. + */ + virDomainLockFree(lock); + + return ret; } static int @@ -3864,11 +3897,13 @@ static int qemudStartVMDaemon(virConnectPtr conn, int logfile = -1; char *timestamp; qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainLockPtr lock = NULL; struct qemudHookData hookData; hookData.conn = conn; hookData.vm = vm; hookData.driver = driver; + hookData.lockState = NULL; /* XXX add lock state from migration source ! */ FD_ZERO(&keepfd); @@ -3902,11 +3937,6 @@ static int qemudStartVMDaemon(virConnectPtr conn, } qemuDomainSecurityLabelAudit(vm, true); - DEBUG0("Generating setting domain security labels (if required)"); - if (virSecurityManagerSetAllLabel(driver->securityManager, - vm, stdin_path) < 0) - goto cleanup; - /* Ensure no historical cgroup for this VM is lying around bogus * settings */ DEBUG0("Ensuring no historical cgroup is lying around"); @@ -4117,13 +4147,33 @@ static int qemudStartVMDaemon(virConnectPtr conn, runflags |= VIR_EXEC_CLEAR_CAPS; } + VIR_WARN("Executing %s", argv[0]); ret = virExecDaemonize(argv, progenv, &keepfd, &child, stdin_fd, &logfile, &logfile, runflags, qemudSecurityHook, &hookData, pidfile); + VIR_WARN("Executing done %s", argv[0]); VIR_FREE(pidfile); + /* XXX this is bollocks. Need a sync point */ + sleep(5); + + VIR_WARN("Locking %s", argv[0]); + if (!(lock = virDomainLockForStartup(driver->contentLockManager, + driver->metadataLockManager, + NULL, /* XXX lock state */ + vm))) + goto cleanup; + + VIR_WARN("Labelling %s", argv[0]); + DEBUG0("Setting domain security labels"); + if (virSecurityManagerSetAllLabel(driver->securityManager, + vm, stdin_path) < 0) + goto cleanup; + VIR_WARN("All done %s", argv[0]); + + /* wait for qemu process to to show up */ if (ret == 0) { if (virFileReadPid(driver->stateDir, vm->def->name, &vm->pid)) { @@ -4213,11 +4263,18 @@ static int qemudStartVMDaemon(virConnectPtr conn, if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) goto cleanup; + virDomainLockReleaseAndFree(lock); + VIR_FORCE_CLOSE(logfile); return 0; cleanup: + /* Must release before we call into ShutdownVMDaemon() because + * that will re-aquire the lock in order to perform relabelling + */ + virDomainLockReleaseAndFree(lock); + /* We jump here if we failed to start the VM for any reason, or * if we failed to initialize the now running VM. kill it off and * pretend we never started it */ @@ -4320,8 +4377,14 @@ static void qemudShutdownVMDaemon(struct qemud_driver *driver, } /* Reset Security Labels */ - virSecurityManagerRestoreAllLabel(driver->securityManager, - vm, migrated); + virDomainLockPtr lock = NULL; + if ((lock = virDomainLockForShutdown(driver->contentLockManager, + driver->metadataLockManager, + vm)) != NULL) { + virSecurityManagerRestoreAllLabel(driver->securityManager, + vm, migrated); + virDomainLockReleaseAndFree(lock); + } virSecurityManagerReleaseLabel(driver->securityManager, vm); @@ -7709,6 +7772,8 @@ static int qemudDomainChangeEjectableMedia(struct qemud_driver *driver, int i; int ret; char *driveAlias = NULL; + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainLockPtr lock; origdisk = NULL; for (i = 0 ; i < vm->def->ndisks ; i++) { @@ -7741,14 +7806,28 @@ static int qemudDomainChangeEjectableMedia(struct qemud_driver *driver, return -1; } + if (!(lock = virDomainLockForModify(driver->contentLockManager, + driver->metadataLockManager, + vm))) + return -1; + + if (virDomainLockBeginDiskAttach(lock, disk) < 0) { + virDomainLockReleaseAndFree(lock); + return -1; + } + if (virSecurityManagerSetImageLabel(driver->securityManager, - vm, disk) < 0) + vm, disk) < 0) { + virDomainLockReleaseAndFree(lock); return -1; + } + + if (virDomainLockEndDiskAttach(lock, disk) < 0) + VIR_WARN("Unable to release lock on disk %s", disk->src); if (!(driveAlias = qemuDeviceDriveHostAlias(origdisk, qemuCmdFlags))) goto error; - qemuDomainObjPrivatePtr priv = vm->privateData; qemuDomainObjEnterMonitorWithDriver(driver, vm); if (disk->src) { const char *format = NULL; @@ -7771,9 +7850,13 @@ static int qemudDomainChangeEjectableMedia(struct qemud_driver *driver, if (ret < 0) goto error; - if (virSecurityManagerRestoreImageLabel(driver->securityManager, - vm, origdisk) < 0) - VIR_WARN("Unable to restore security label on ejected image %s", origdisk->src); + if (virDomainLockBeginDiskDetach(lock, origdisk) >= 0) { + if (virSecurityManagerRestoreImageLabel(driver->securityManager, + vm, origdisk) < 0) + VIR_WARN("Unable to restore security label on ejected image %s", origdisk->src); + if (virDomainLockEndDiskDetach(lock, origdisk) < 0) + VIR_WARN("Unable to release lock on disk %s", origdisk->src); + } VIR_FREE(origdisk->src); origdisk->src = disk->src; @@ -7783,14 +7866,21 @@ static int qemudDomainChangeEjectableMedia(struct qemud_driver *driver, VIR_FREE(driveAlias); virDomainDiskDefFree(disk); + virDomainLockReleaseAndFree(lock); return ret; error: VIR_FREE(driveAlias); - if (virSecurityManagerRestoreImageLabel(driver->securityManager, - vm, disk) < 0) - VIR_WARN("Unable to restore security label on new media %s", disk->src); + + if (virDomainLockBeginDiskDetach(lock, disk) >= 0) { + if (virSecurityManagerRestoreImageLabel(driver->securityManager, + vm, disk) < 0) + VIR_WARN("Unable to restore security label on new media %s", disk->src); + if (virDomainLockEndDiskDetach(lock, disk) < 0) + VIR_WARN("Unable to release lock on disk %s", disk->src); + } + virDomainLockReleaseAndFree(lock); return -1; } @@ -7806,6 +7896,7 @@ static int qemudDomainAttachPciDiskDevice(struct qemud_driver *driver, qemuDomainObjPrivatePtr priv = vm->privateData; char *devstr = NULL; char *drivestr = NULL; + virDomainLockPtr lock = NULL; for (i = 0 ; i < vm->def->ndisks ; i++) { if (STREQ(vm->def->disks[i]->dst, disk->dst)) { @@ -7815,9 +7906,24 @@ static int qemudDomainAttachPciDiskDevice(struct qemud_driver *driver, } } + if (!(lock = virDomainLockForModify(driver->contentLockManager, + driver->metadataLockManager, + vm))) + return -1; + + if (virDomainLockBeginDiskAttach(lock, disk) < 0) { + virDomainLockReleaseAndFree(lock); + return -1; + } + if (virSecurityManagerSetImageLabel(driver->securityManager, - vm, disk) < 0) + vm, disk) < 0) { + virDomainLockReleaseAndFree(lock); return -1; + } + + if (virDomainLockEndDiskAttach(lock, disk) < 0) + VIR_WARN("Unable to release lock on disk %s", disk->src); if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &disk->info) < 0) @@ -7871,6 +7977,7 @@ static int qemudDomainAttachPciDiskDevice(struct qemud_driver *driver, VIR_FREE(devstr); VIR_FREE(drivestr); + virDomainLockReleaseAndFree(lock); return 0; @@ -7883,9 +7990,14 @@ error: qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &disk->info) < 0) VIR_WARN("Unable to release PCI address on %s", disk->src); - if (virSecurityManagerRestoreImageLabel(driver->securityManager, - vm, disk) < 0) - VIR_WARN("Unable to restore security label on %s", disk->src); + if (virDomainLockBeginDiskDetach(lock, disk) >= 0) { + if (virSecurityManagerRestoreImageLabel(driver->securityManager, + vm, disk) < 0) + VIR_WARN("Unable to restore security label on %s", disk->src); + if (virDomainLockEndDiskDetach(lock, disk) < 0) + VIR_WARN("Unable to release lock on disk %s", disk->src); + } + virDomainLockReleaseAndFree(lock); return -1; } @@ -8014,6 +8126,7 @@ static int qemudDomainAttachSCSIDisk(struct qemud_driver *driver, char *drivestr = NULL; char *devstr = NULL; int ret = -1; + virDomainLockPtr lock = NULL; for (i = 0 ; i < vm->def->ndisks ; i++) { if (STREQ(vm->def->disks[i]->dst, disk->dst)) { @@ -8023,10 +8136,24 @@ static int qemudDomainAttachSCSIDisk(struct qemud_driver *driver, } } + if (!(lock = virDomainLockForModify(driver->contentLockManager, + driver->metadataLockManager, + vm))) + return -1; + + if (virDomainLockBeginDiskAttach(lock, disk) < 0) { + virDomainLockReleaseAndFree(lock); + return -1; + } if (virSecurityManagerSetImageLabel(driver->securityManager, - vm, disk) < 0) + vm, disk) < 0) { + virDomainLockReleaseAndFree(lock); return -1; + } + + if (virDomainLockEndDiskAttach(lock, disk) < 0) + VIR_WARN("Unable to release lock on disk %s", disk->src); /* We should have an address already, so make sure */ if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { @@ -8104,6 +8231,7 @@ static int qemudDomainAttachSCSIDisk(struct qemud_driver *driver, VIR_FREE(devstr); VIR_FREE(drivestr); + virDomainLockReleaseAndFree(lock); return 0; @@ -8111,9 +8239,14 @@ error: VIR_FREE(devstr); VIR_FREE(drivestr); - if (virSecurityManagerRestoreImageLabel(driver->securityManager, - vm, disk) < 0) - VIR_WARN("Unable to restore security label on %s", disk->src); + if (virDomainLockBeginDiskDetach(lock, disk) >= 0) { + if (virSecurityManagerRestoreImageLabel(driver->securityManager, + vm, disk) < 0) + VIR_WARN("Unable to restore security label on %s", disk->src); + if (virDomainLockEndDiskDetach(lock, disk) < 0) + VIR_WARN("Unable to release lock on disk %s", disk->src); + } + virDomainLockReleaseAndFree(lock); return -1; } @@ -8128,6 +8261,7 @@ static int qemudDomainAttachUsbMassstorageDevice(struct qemud_driver *driver, int i, ret; char *drivestr = NULL; char *devstr = NULL; + virDomainLockPtr lock = NULL; for (i = 0 ; i < vm->def->ndisks ; i++) { if (STREQ(vm->def->disks[i]->dst, disk->dst)) { @@ -8137,10 +8271,26 @@ static int qemudDomainAttachUsbMassstorageDevice(struct qemud_driver *driver, } } + if (!(lock = virDomainLockForModify(driver->contentLockManager, + driver->metadataLockManager, + vm))) + return -1; + + if (virDomainLockBeginDiskAttach(lock, disk) < 0) { + virDomainLockReleaseAndFree(lock); + return -1; + } + if (virSecurityManagerSetImageLabel(driver->securityManager, - vm, disk) < 0) + vm, disk) < 0) { + virDomainLockReleaseAndFree(lock); return -1; + } + + if (virDomainLockEndDiskAttach(lock, disk) < 0) + VIR_WARN("Unable to release lock on disk %s", disk->src); + /* XXX not correct once we allow attaching a USB CDROM */ if (!disk->src) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("disk source path is missing")); @@ -8187,6 +8337,7 @@ static int qemudDomainAttachUsbMassstorageDevice(struct qemud_driver *driver, VIR_FREE(devstr); VIR_FREE(drivestr); + virDomainLockReleaseAndFree(lock); return 0; @@ -8194,9 +8345,15 @@ error: VIR_FREE(devstr); VIR_FREE(drivestr); - if (virSecurityManagerRestoreImageLabel(driver->securityManager, - vm, disk) < 0) - VIR_WARN("Unable to restore security label on %s", disk->src); + + if (virDomainLockBeginDiskDetach(lock, disk) >= 0) { + if (virSecurityManagerRestoreImageLabel(driver->securityManager, + vm, disk) < 0) + VIR_WARN("Unable to restore security label on %s", disk->src); + if (virDomainLockEndDiskDetach(lock, disk) < 0) + VIR_WARN("Unable to release lock on disk %s", disk->src); + } + virDomainLockReleaseAndFree(lock); return -1; } @@ -9032,6 +9189,12 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, virDomainDiskDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup = NULL; + virDomainLockPtr lock = NULL; + + if (!(lock = virDomainLockForModify(driver->contentLockManager, + driver->metadataLockManager, + vm))) + return -1; i = qemudFindDisk(vm->def, dev->data.disk->dst); @@ -9084,9 +9247,13 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, virDomainDiskDefFree(detach); - if (virSecurityManagerRestoreImageLabel(driver->securityManager, - vm, dev->data.disk) < 0) - VIR_WARN("Unable to restore security label on %s", dev->data.disk->src); + if (virDomainLockBeginDiskDetach(lock, dev->data.disk)) { + if (virSecurityManagerRestoreImageLabel(driver->securityManager, + vm, dev->data.disk) < 0) + VIR_WARN("Unable to restore security label on %s", dev->data.disk->src); + if (virDomainLockEndDiskDetach(lock, dev->data.disk) < 0) + VIR_WARN("Unable to release lock on disk %s", dev->data.disk->src); + } if (cgroup != NULL) { if (qemuTeardownDiskCgroup(driver, cgroup, dev->data.disk) < 0) @@ -9097,6 +9264,7 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, ret = 0; cleanup: + virDomainLockReleaseAndFree(lock); return ret; } @@ -9109,6 +9277,12 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, virDomainDiskDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup = NULL; + virDomainLockPtr lock = NULL; + + if (!(lock = virDomainLockForModify(driver->contentLockManager, + driver->metadataLockManager, + vm))) + return -1; i = qemudFindDisk(vm->def, dev->data.disk->dst); @@ -9148,9 +9322,13 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, virDomainDiskDefFree(detach); - if (virSecurityManagerRestoreImageLabel(driver->securityManager, - vm, dev->data.disk) < 0) - VIR_WARN("Unable to restore security label on %s", dev->data.disk->src); + if (virDomainLockBeginDiskDetach(lock, dev->data.disk)) { + if (virSecurityManagerRestoreImageLabel(driver->securityManager, + vm, dev->data.disk) < 0) + VIR_WARN("Unable to restore security label on %s", dev->data.disk->src); + if (virDomainLockEndDiskDetach(lock, dev->data.disk) < 0) + VIR_WARN("Unable to release lock on disk %s", dev->data.disk->src); + } if (cgroup != NULL) { if (qemuTeardownDiskCgroup(driver, cgroup, dev->data.disk) < 0) @@ -9162,6 +9340,7 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, cleanup: virCgroupFree(&cgroup); + virDomainLockReleaseAndFree(lock); return ret; } diff --git a/src/qemu/test_libvirtd_qemu.aug b/src/qemu/test_libvirtd_qemu.aug index d3ae58d..fd56eb9 100644 --- a/src/qemu/test_libvirtd_qemu.aug +++ b/src/qemu/test_libvirtd_qemu.aug @@ -107,6 +107,8 @@ vnc_allow_host_audio = 1 clear_emulator_capabilities = 0 allow_disk_format_probing = 1 + +lock_manager = \"fcntl\" " test Libvirtd_qemu.lns get conf = @@ -224,3 +226,5 @@ allow_disk_format_probing = 1 { "clear_emulator_capabilities" = "0" } { "#empty" } { "allow_disk_format_probing" = "1" } +{ "#empty" } +{ "lock_manager" = "fcntl" } -- 1.7.2.3

Switch the QEMU driver over to using the virCommand APIs instead of passing around char **argv, **env. * src/qemu/qemu_conf.c, src/qemu/qemu_conf.h, src/qemu/qemu_driver.c: Convert to virCommand --- src/locking/lock_manager.c | 2 + src/qemu/qemu_conf.c | 632 +++++++++++++++++++------------------------- src/qemu/qemu_conf.h | 21 +- src/qemu/qemu_driver.c | 195 +++----------- 4 files changed, 324 insertions(+), 526 deletions(-) diff --git a/src/locking/lock_manager.c b/src/locking/lock_manager.c index dc5e7d8..6bd2844 100644 --- a/src/locking/lock_manager.c +++ b/src/locking/lock_manager.c @@ -32,6 +32,8 @@ #include <stdlib.h> #include <unistd.h> +#include "configmake.h" + #define VIR_FROM_THIS VIR_FROM_LOCKING #define virLockError(code, ...) \ diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index dce9a63..ec255c9 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1554,12 +1554,24 @@ int qemudExtractVersionInfo(const char *qemu, int ret = -1, status; unsigned int version, is_kvm, kvm_version; unsigned long long flags = 0; + struct stat sb; if (retflags) *retflags = 0; if (retversion) *retversion = 0; + /* Make sure the binary we are about to try exec'ing exists. + * Technically we could catch the exec() failure, but that's + * in a sub-process so its hard to feed back a useful error + */ + if (stat(qemu, &sb) < 0) { + virReportSystemError(errno, + _("Cannot find QEMU binary %s"), + qemu); + return -1; + } + if (virExec(qemuarg, qemuenv, NULL, &child, -1, &newstdout, NULL, VIR_EXEC_CLEAR_CAPS) < 0) return -1; @@ -3972,18 +3984,14 @@ qemuBuildSmpArgStr(const virDomainDefPtr def, * XXX 'conn' is only required to resolve network -> bridge name * figure out how to remove this requirement some day */ -int qemudBuildCommandLine(virConnectPtr conn, - struct qemud_driver *driver, - virDomainDefPtr def, - virDomainChrDefPtr monitor_chr, - int monitor_json, - unsigned long long qemuCmdFlags, - const char ***retargv, - const char ***retenv, - int **vmfds, - int *nvmfds, - const char *migrateFrom, - virDomainSnapshotObjPtr current_snapshot) +virCommandPtr qemudBuildCommandLine(virConnectPtr conn, + struct qemud_driver *driver, + virDomainDefPtr def, + virDomainChrDefPtr monitor_chr, + int monitor_json, + unsigned long long qemuCmdFlags, + const char *migrateFrom, + virDomainSnapshotObjPtr current_snapshot) { int i; char memory[50]; @@ -3993,10 +4001,6 @@ int qemudBuildCommandLine(virConnectPtr conn, int enableKQEMU = 0; int disableKVM = 0; int enableKVM = 0; - int qargc = 0, qarga = 0; - const char **qargv = NULL; - int qenvc = 0, qenva = 0; - const char **qenv = NULL; const char *emulator; char uuid[VIR_UUID_STRING_BUFLEN]; char domid[50]; @@ -4004,11 +4008,12 @@ int qemudBuildCommandLine(virConnectPtr conn, char *smp; int last_good_net = -1; bool hasHwVirt = false; + virCommandPtr cmd; uname_normalize(&ut); if (qemuAssignDeviceAliases(def, qemuCmdFlags) < 0) - return -1; + return NULL; virUUIDFormat(def->uuid, uuid); @@ -4020,7 +4025,7 @@ int qemudBuildCommandLine(virConnectPtr conn, if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP)) { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("TCP migration is not supported with this QEMU binary")); - return -1; + return NULL; } } else if (STREQ(migrateFrom, "stdio")) { if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC) { @@ -4028,13 +4033,13 @@ int qemudBuildCommandLine(virConnectPtr conn, } else if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_KVM_STDIO)) { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("STDIO migration is not supported with this QEMU binary")); - return -1; + return NULL; } } else if (STRPREFIX(migrateFrom, "exec")) { if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC)) { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("STDIO migration is not supported with this QEMU binary")); - return -1; + return NULL; } } } @@ -4095,78 +4100,11 @@ int qemudBuildCommandLine(virConnectPtr conn, break; } -#define ADD_ARG_SPACE \ - do { \ - if (qargc == qarga) { \ - qarga += 10; \ - if (VIR_REALLOC_N(qargv, qarga) < 0) \ - goto no_memory; \ - } \ - } while (0) - -#define ADD_ARG(thisarg) \ - do { \ - ADD_ARG_SPACE; \ - qargv[qargc++] = thisarg; \ - } while (0) - -#define ADD_ARG_LIT(thisarg) \ - do { \ - ADD_ARG_SPACE; \ - if ((qargv[qargc++] = strdup(thisarg)) == NULL) \ - goto no_memory; \ - } while (0) - -#define ADD_USBDISK(thisarg) \ - do { \ - ADD_ARG_LIT("-usbdevice"); \ - ADD_ARG_SPACE; \ - if ((virAsprintf((char **)&(qargv[qargc++]), \ - "disk:%s", thisarg)) == -1) { \ - goto no_memory; \ - } \ - } while (0) - -#define ADD_ENV_SPACE \ - do { \ - if (qenvc == qenva) { \ - qenva += 10; \ - if (VIR_REALLOC_N(qenv, qenva) < 0) \ - goto no_memory; \ - } \ - } while (0) - -#define ADD_ENV(thisarg) \ - do { \ - ADD_ENV_SPACE; \ - qenv[qenvc++] = thisarg; \ - } while (0) - -#define ADD_ENV_LIT(thisarg) \ - do { \ - ADD_ENV_SPACE; \ - if ((qenv[qenvc++] = strdup(thisarg)) == NULL) \ - goto no_memory; \ - } while (0) - -#define ADD_ENV_PAIR(envname, val) \ - do { \ - char *envval; \ - ADD_ENV_SPACE; \ - if (virAsprintf(&envval, "%s=%s", envname, val) < 0) \ - goto no_memory; \ - qenv[qenvc++] = envval; \ - } while (0) - - /* Make sure to unset or set all envvars in qemuxml2argvtest.c that - * are copied here using this macro, otherwise the test may fail */ -#define ADD_ENV_COPY(envname) \ - do { \ - char *val = getenv(envname); \ - if (val != NULL) { \ - ADD_ENV_PAIR(envname, val); \ - } \ - } while (0) + cmd = virCommandNew(emulator); + if (!cmd) { + virReportOOMError(); + return NULL; + } /* Set '-m MB' based on maxmem, because the lower 'memory' limit * is set post-startup using the balloon driver. If balloon driver @@ -4175,26 +4113,17 @@ int qemudBuildCommandLine(virConnectPtr conn, snprintf(memory, sizeof(memory), "%lu", def->mem.max_balloon/1024); snprintf(domid, sizeof(domid), "%d", def->id); - ADD_ENV_LIT("LC_ALL=C"); + virCommandAddEnvPassCommon(cmd); - ADD_ENV_COPY("LD_PRELOAD"); - ADD_ENV_COPY("LD_LIBRARY_PATH"); - ADD_ENV_COPY("PATH"); - ADD_ENV_COPY("HOME"); - ADD_ENV_COPY("USER"); - ADD_ENV_COPY("LOGNAME"); - ADD_ENV_COPY("TMPDIR"); - - ADD_ARG_LIT(emulator); - ADD_ARG_LIT("-S"); + virCommandAddArg(cmd, "-S"); /* This should *never* be NULL, since we always provide * a machine in the capabilities data for QEMU. So this * check is just here as a safety in case the unexpected * happens */ if (def->os.machine) { - ADD_ARG_LIT("-M"); - ADD_ARG_LIT(def->os.machine); + virCommandAddArg(cmd, "-M"); + virCommandAddArg(cmd, def->os.machine); } if (qemuBuildCpuArgStr(driver, def, emulator, qemuCmdFlags, @@ -4202,27 +4131,27 @@ int qemudBuildCommandLine(virConnectPtr conn, goto error; if (cpu) { - ADD_ARG_LIT("-cpu"); - ADD_ARG_LIT(cpu); + virCommandAddArg(cmd, "-cpu"); + virCommandAddArg(cmd, cpu); VIR_FREE(cpu); if ((qemuCmdFlags & QEMUD_CMD_FLAG_NESTING) && hasHwVirt) - ADD_ARG_LIT("-enable-nesting"); + virCommandAddArg(cmd, "-enable-nesting"); } if (disableKQEMU) - ADD_ARG_LIT("-no-kqemu"); + virCommandAddArg(cmd, "-no-kqemu"); else if (enableKQEMU) { - ADD_ARG_LIT("-enable-kqemu"); - ADD_ARG_LIT("-kernel-kqemu"); + virCommandAddArg(cmd, "-enable-kqemu"); + virCommandAddArg(cmd, "-kernel-kqemu"); } if (disableKVM) - ADD_ARG_LIT("-no-kvm"); + virCommandAddArg(cmd, "-no-kvm"); if (enableKVM) - ADD_ARG_LIT("-enable-kvm"); - ADD_ARG_LIT("-m"); - ADD_ARG_LIT(memory); + virCommandAddArg(cmd, "-enable-kvm"); + virCommandAddArg(cmd, "-m"); + virCommandAddArg(cmd, memory); if (def->mem.hugepage_backed) { if (!driver->hugetlbfs_mount) { qemuReportError(VIR_ERR_INTERNAL_ERROR, @@ -4240,43 +4169,44 @@ int qemudBuildCommandLine(virConnectPtr conn, def->emulator); goto error; } - ADD_ARG_LIT("-mem-prealloc"); - ADD_ARG_LIT("-mem-path"); - ADD_ARG_LIT(driver->hugepage_path); + virCommandAddArg(cmd, "-mem-prealloc"); + virCommandAddArg(cmd, "-mem-path"); + virCommandAddArg(cmd, driver->hugepage_path); } - ADD_ARG_LIT("-smp"); + virCommandAddArg(cmd, "-smp"); if (!(smp = qemuBuildSmpArgStr(def, qemuCmdFlags))) goto error; - ADD_ARG(smp); + virCommandAddArg(cmd, smp); + VIR_FREE(smp); if (qemuCmdFlags & QEMUD_CMD_FLAG_NAME) { - ADD_ARG_LIT("-name"); + virCommandAddArg(cmd, "-name"); if (driver->setProcessName && (qemuCmdFlags & QEMUD_CMD_FLAG_NAME_PROCESS)) { char *name; if (virAsprintf(&name, "%s,process=qemu:%s", def->name, def->name) < 0) goto no_memory; - ADD_ARG_LIT(name); + virCommandAddArg(cmd, name); } else { - ADD_ARG_LIT(def->name); + virCommandAddArg(cmd, def->name); } } if (qemuCmdFlags & QEMUD_CMD_FLAG_UUID) { - ADD_ARG_LIT("-uuid"); - ADD_ARG_LIT(uuid); + virCommandAddArg(cmd, "-uuid"); + virCommandAddArg(cmd, uuid); } if (def->virtType == VIR_DOMAIN_VIRT_XEN || STREQ(def->os.type, "xen") || STREQ(def->os.type, "linux")) { if (qemuCmdFlags & QEMUD_CMD_FLAG_DOMID) { - ADD_ARG_LIT("-domid"); - ADD_ARG_LIT(domid); + virCommandAddArg(cmd, "-domid"); + virCommandAddArg(cmd, domid); } else if (qemuCmdFlags & QEMUD_CMD_FLAG_XEN_DOMID) { - ADD_ARG_LIT("-xen-attach"); - ADD_ARG_LIT("-xen-domid"); - ADD_ARG_LIT(domid); + virCommandAddArg(cmd, "-xen-attach"); + virCommandAddArg(cmd, "-xen-domid"); + virCommandAddArg(cmd, domid); } else { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("qemu emulator '%s' does not support xen"), @@ -4318,13 +4248,15 @@ int qemudBuildCommandLine(virConnectPtr conn, smbioscmd = qemuBuildSmbiosBiosStr(source); if (smbioscmd != NULL) { - ADD_ARG_LIT("-smbios"); - ADD_ARG(smbioscmd); + virCommandAddArg(cmd, "-smbios"); + virCommandAddArg(cmd, smbioscmd); + VIR_FREE(smbioscmd); } smbioscmd = qemuBuildSmbiosSystemStr(source); if (smbioscmd != NULL) { - ADD_ARG_LIT("-smbios"); - ADD_ARG(smbioscmd); + virCommandAddArg(cmd, "-smbios"); + virCommandAddArg(cmd, smbioscmd); + VIR_FREE(smbioscmd); } } } @@ -4337,12 +4269,12 @@ int qemudBuildCommandLine(virConnectPtr conn, * these defaults ourselves... */ if (!def->graphics) - ADD_ARG_LIT("-nographic"); + virCommandAddArg(cmd, "-nographic"); if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { if (qemuCmdFlags & QEMUD_CMD_FLAG_NODEFCONFIG) - ADD_ARG_LIT("-nodefconfig"); /* Disabling global config files */ - ADD_ARG_LIT("-nodefaults"); /* Disabling default guest devices */ + virCommandAddArg(cmd, "-nodefconfig"); /* Disabling global config files */ + virCommandAddArg(cmd, "-nodefaults"); /* Disabling default guest devices */ } if (monitor_chr) { @@ -4350,39 +4282,42 @@ int qemudBuildCommandLine(virConnectPtr conn, /* Use -chardev if it's available */ if (qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) { - ADD_ARG_LIT("-chardev"); + virCommandAddArg(cmd, "-chardev"); if (!(chrdev = qemuBuildChrChardevStr(monitor_chr))) goto error; - ADD_ARG(chrdev); + virCommandAddArg(cmd, chrdev); + VIR_FREE(chrdev); - ADD_ARG_LIT("-mon"); + virCommandAddArg(cmd, "-mon"); if (monitor_json) - ADD_ARG_LIT("chardev=monitor,mode=control"); + virCommandAddArg(cmd, "chardev=monitor,mode=control"); else - ADD_ARG_LIT("chardev=monitor,mode=readline"); + virCommandAddArg(cmd, "chardev=monitor,mode=readline"); } else { const char *prefix = NULL; if (monitor_json) prefix = "control,"; - ADD_ARG_LIT("-monitor"); + virCommandAddArg(cmd, "-monitor"); if (!(chrdev = qemuBuildChrArgStr(monitor_chr, prefix))) goto error; - ADD_ARG(chrdev); + virCommandAddArg(cmd, chrdev); + VIR_FREE(chrdev); } } if (qemuCmdFlags & QEMUD_CMD_FLAG_RTC) { const char *rtcopt; - ADD_ARG_LIT("-rtc"); + virCommandAddArg(cmd, "-rtc"); if (!(rtcopt = qemuBuildClockArgStr(&def->clock))) goto error; - ADD_ARG(rtcopt); + virCommandAddArg(cmd, rtcopt); + VIR_FREE(rtcopt); } else { switch (def->clock.offset) { case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME: case VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE: - ADD_ARG_LIT("-localtime"); + virCommandAddArg(cmd, "-localtime"); break; case VIR_DOMAIN_CLOCK_OFFSET_UTC: @@ -4397,9 +4332,8 @@ int qemudBuildCommandLine(virConnectPtr conn, } } if (def->clock.offset == VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE && - def->clock.data.timezone) { - ADD_ENV_PAIR("TZ", def->clock.data.timezone); - } + def->clock.data.timezone) + virCommandAddEnvPair(cmd, "TZ", def->clock.data.timezone); for (i = 0; i < def->clock.ntimers; i++) { switch (def->clock.timers[i]->name) { @@ -4422,7 +4356,7 @@ int qemudBuildCommandLine(virConnectPtr conn, /* the default - do nothing */ break; case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP: - ADD_ARG_LIT("-rtc-td-hack"); + virCommandAddArg(cmd, "-rtc-td-hack"); break; case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD: @@ -4451,14 +4385,14 @@ int qemudBuildCommandLine(virConnectPtr conn, /* delay is the default if we don't have kernel (-no-kvm-pit), otherwise, the default is catchup. */ if (qemuCmdFlags & QEMUD_CMD_FLAG_NO_KVM_PIT) - ADD_ARG_LIT("-no-kvm-pit-reinjection"); + virCommandAddArg(cmd, "-no-kvm-pit-reinjection"); break; case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP: if (qemuCmdFlags & QEMUD_CMD_FLAG_NO_KVM_PIT) { /* do nothing - this is default for kvm-pit */ } else if (qemuCmdFlags & QEMUD_CMD_FLAG_TDF) { /* -tdf switches to 'catchup' with userspace pit. */ - ADD_ARG_LIT("-tdf"); + virCommandAddArg(cmd, "-tdf"); } else { /* can't catchup if we have neither pit mode */ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -4487,7 +4421,7 @@ int qemudBuildCommandLine(virConnectPtr conn, if (qemuCmdFlags & QEMUD_CMD_FLAG_NO_HPET) { if (def->clock.timers[i]->present == 0) - ADD_ARG_LIT("-no-hpet"); + virCommandAddArg(cmd, "-no-hpet"); } else { /* no hpet timer available. The only possible action is to raise an error if present="yes" */ @@ -4502,10 +4436,10 @@ int qemudBuildCommandLine(virConnectPtr conn, if ((qemuCmdFlags & QEMUD_CMD_FLAG_NO_REBOOT) && def->onReboot != VIR_DOMAIN_LIFECYCLE_RESTART) - ADD_ARG_LIT("-no-reboot"); + virCommandAddArg(cmd, "-no-reboot"); if (!(def->features & (1 << VIR_DOMAIN_FEATURE_ACPI))) - ADD_ARG_LIT("-no-acpi"); + virCommandAddArg(cmd, "-no-acpi"); if (!def->os.bootloader) { for (i = 0 ; i < def->os.nBootDevs ; i++) { @@ -4529,7 +4463,8 @@ int qemudBuildCommandLine(virConnectPtr conn, } if (def->os.nBootDevs) { virBuffer boot_buf = VIR_BUFFER_INITIALIZER; - ADD_ARG_LIT("-boot"); + char *bootstr; + virCommandAddArg(cmd, "-boot"); boot[def->os.nBootDevs] = '\0'; @@ -4548,24 +4483,26 @@ int qemudBuildCommandLine(virConnectPtr conn, goto error; } - ADD_ARG(virBufferContentAndReset(&boot_buf)); + bootstr = virBufferContentAndReset(&boot_buf); + virCommandAddArg(cmd, bootstr); + VIR_FREE(bootstr); } if (def->os.kernel) { - ADD_ARG_LIT("-kernel"); - ADD_ARG_LIT(def->os.kernel); + virCommandAddArg(cmd, "-kernel"); + virCommandAddArg(cmd, def->os.kernel); } if (def->os.initrd) { - ADD_ARG_LIT("-initrd"); - ADD_ARG_LIT(def->os.initrd); + virCommandAddArg(cmd, "-initrd"); + virCommandAddArg(cmd, def->os.initrd); } if (def->os.cmdline) { - ADD_ARG_LIT("-append"); - ADD_ARG_LIT(def->os.cmdline); + virCommandAddArg(cmd, "-append"); + virCommandAddArg(cmd, def->os.cmdline); } } else { - ADD_ARG_LIT("-bootloader"); - ADD_ARG_LIT(def->os.bootloader); + virCommandAddArg(cmd, "-bootloader"); + virCommandAddArg(cmd, def->os.bootloader); } for (i = 0 ; i < def->ndisks ; i++) { @@ -4598,13 +4535,14 @@ int qemudBuildCommandLine(virConnectPtr conn, goto error; } - ADD_ARG_LIT("-device"); + virCommandAddArg(cmd, "-device"); char *devstr; if (!(devstr = qemuBuildControllerDevStr(def->controllers[i]))) goto no_memory; - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); } } @@ -4640,7 +4578,8 @@ int qemudBuildCommandLine(virConnectPtr conn, if ((disk->bus == VIR_DOMAIN_DISK_BUS_USB) && !(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - ADD_USBDISK(disk->src); + virCommandAddArg(cmd, "-usbdevice"); + virCommandAddArg(cmd, disk->src); } else { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported usb disk type for '%s'"), disk->src); @@ -4664,7 +4603,7 @@ int qemudBuildCommandLine(virConnectPtr conn, break; } - ADD_ARG_LIT("-drive"); + virCommandAddArg(cmd, "-drive"); /* Unfortunately it is nt possible to use -device for floppys, or Xen paravirt @@ -4678,24 +4617,27 @@ int qemudBuildCommandLine(virConnectPtr conn, (withDeviceArg ? qemuCmdFlags : (qemuCmdFlags & ~QEMUD_CMD_FLAG_DEVICE))))) goto error; - ADD_ARG(optstr); + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); if (withDeviceArg) { if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC) { char *fdc; - ADD_ARG_LIT("-global"); + virCommandAddArg(cmd, "-global"); if (virAsprintf(&fdc, "isa-fdc.drive%c=drive-%s", disk->info.addr.drive.unit ? 'B' : 'A', disk->info.alias) < 0) goto no_memory; - ADD_ARG(fdc); + virCommandAddArg(cmd, fdc); + VIR_FREE(fdc); } else { - ADD_ARG_LIT("-device"); + virCommandAddArg(cmd, "-device"); if (!(optstr = qemuBuildDriveDevStr(disk))) goto error; - ADD_ARG(optstr); + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); } } } @@ -4707,7 +4649,8 @@ int qemudBuildCommandLine(virConnectPtr conn, if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - ADD_USBDISK(disk->src); + virCommandAddArg(cmd, "-usbdevice"); + virCommandAddArg(cmd, disk->src); } else { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported usb disk type for '%s'"), disk->src); @@ -4756,8 +4699,8 @@ int qemudBuildCommandLine(virConnectPtr conn, snprintf(file, PATH_MAX, "%s", disk->src); } - ADD_ARG_LIT(dev); - ADD_ARG_LIT(file); + virCommandAddArg(cmd, dev); + virCommandAddArg(cmd, file); } } @@ -4766,15 +4709,17 @@ int qemudBuildCommandLine(virConnectPtr conn, char *optstr; virDomainFSDefPtr fs = def->fss[i]; - ADD_ARG_LIT("-fsdev"); + virCommandAddArg(cmd, "-fsdev"); if (!(optstr = qemuBuildFSStr(fs, qemuCmdFlags))) goto error; - ADD_ARG(optstr); + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); - ADD_ARG_LIT("-device"); + virCommandAddArg(cmd, "-device"); if (!(optstr = qemuBuildFSDevStr(fs))) goto error; - ADD_ARG(optstr); + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); } } else { if (def->nfss) { @@ -4787,8 +4732,8 @@ int qemudBuildCommandLine(virConnectPtr conn, if (!def->nnets) { /* If we have -device, then we set -nodefault already */ if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { - ADD_ARG_LIT("-net"); - ADD_ARG_LIT("none"); + virCommandAddArg(cmd, "-net"); + virCommandAddArg(cmd, "none"); } } else { for (i = 0 ; i < def->nnets ; i++) { @@ -4811,15 +4756,9 @@ int qemudBuildCommandLine(virConnectPtr conn, if (tapfd < 0) goto error; - if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) { - virDomainConfNWFilterTeardown(net); - VIR_FORCE_CLOSE(tapfd); - goto no_memory; - } - last_good_net = i; - (*vmfds)[(*nvmfds)++] = tapfd; + virCommandPreserveFD(cmd, tapfd); if (snprintf(tapfd_name, sizeof(tapfd_name), "%d", tapfd) >= sizeof(tapfd_name)) goto no_memory; @@ -4830,15 +4769,9 @@ int qemudBuildCommandLine(virConnectPtr conn, if (tapfd < 0) goto error; - if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) { - virDomainConfNWFilterTeardown(net); - VIR_FORCE_CLOSE(tapfd); - goto no_memory; - } - last_good_net = i; - (*vmfds)[(*nvmfds)++] = tapfd; + virCommandPreserveFD(cmd, tapfd); if (snprintf(tapfd_name, sizeof(tapfd_name), "%d", tapfd) >= sizeof(tapfd_name)) goto no_memory; @@ -4851,12 +4784,8 @@ int qemudBuildCommandLine(virConnectPtr conn, network device */ int vhostfd = qemudOpenVhostNet(net, qemuCmdFlags); if (vhostfd >= 0) { - if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) { - VIR_FORCE_CLOSE(vhostfd); - goto no_memory; - } + virCommandPreserveFD(cmd, vhostfd); - (*vmfds)[(*nvmfds)++] = vhostfd; if (snprintf(vhostfd_name, sizeof(vhostfd_name), "%d", vhostfd) goto no_memory; @@ -4872,30 +4801,34 @@ int qemudBuildCommandLine(virConnectPtr conn, */ if ((qemuCmdFlags & QEMUD_CMD_FLAG_NETDEV) && (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { - ADD_ARG_LIT("-netdev"); + virCommandAddArg(cmd, "-netdev"); if (!(host = qemuBuildHostNetStr(net, ',', vlan, tapfd_name, vhostfd_name))) goto error; - ADD_ARG(host); + virCommandAddArg(cmd, host); + VIR_FREE(host); } if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { - ADD_ARG_LIT("-device"); + virCommandAddArg(cmd, "-device"); if (!(nic = qemuBuildNicDevStr(net, vlan))) goto error; - ADD_ARG(nic); + virCommandAddArg(cmd, nic); + VIR_FREE(nic); } else { - ADD_ARG_LIT("-net"); + virCommandAddArg(cmd, "-net"); if (!(nic = qemuBuildNicStr(net, "nic,", vlan))) goto error; - ADD_ARG(nic); + virCommandAddArg(cmd, nic); + VIR_FREE(nic); } if (!((qemuCmdFlags & QEMUD_CMD_FLAG_NETDEV) && (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE))) { - ADD_ARG_LIT("-net"); + virCommandAddArg(cmd, "-net"); if (!(host = qemuBuildHostNetStr(net, ',', vlan, tapfd_name, vhostfd_name))) goto error; - ADD_ARG(host); + virCommandAddArg(cmd, host); + VIR_FREE(host); } } } @@ -4903,8 +4836,8 @@ int qemudBuildCommandLine(virConnectPtr conn, if (!def->nserials) { /* If we have -device, then we set -nodefault already */ if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { - ADD_ARG_LIT("-serial"); - ADD_ARG_LIT("none"); + virCommandAddArg(cmd, "-serial"); + virCommandAddArg(cmd, "none"); } } else { for (i = 0 ; i < def->nserials ; i++) { @@ -4914,20 +4847,23 @@ int qemudBuildCommandLine(virConnectPtr conn, /* Use -chardev with -device if they are available */ if ((qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) && (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { - ADD_ARG_LIT("-chardev"); + virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(serial))) goto error; - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); - ADD_ARG_LIT("-device"); + virCommandAddArg(cmd, "-device"); if (virAsprintf(&devstr, "isa-serial,chardev=%s", serial->info.alias) < 0) goto no_memory; - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); } else { - ADD_ARG_LIT("-serial"); + virCommandAddArg(cmd, "-serial"); if (!(devstr = qemuBuildChrArgStr(serial, NULL))) goto error; - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); } } } @@ -4935,8 +4871,8 @@ int qemudBuildCommandLine(virConnectPtr conn, if (!def->nparallels) { /* If we have -device, then we set -nodefault already */ if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { - ADD_ARG_LIT("-parallel"); - ADD_ARG_LIT("none"); + virCommandAddArg(cmd, "-parallel"); + virCommandAddArg(cmd, "none"); } } else { for (i = 0 ; i < def->nparallels ; i++) { @@ -4946,20 +4882,23 @@ int qemudBuildCommandLine(virConnectPtr conn, /* Use -chardev with -device if they are available */ if ((qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) && (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { - ADD_ARG_LIT("-chardev"); + virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(parallel))) goto error; - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); - ADD_ARG_LIT("-device"); + virCommandAddArg(cmd, "-device"); if (virAsprintf(&devstr, "isa-parallel,chardev=%s", parallel->info.alias) < 0) goto no_memory; - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); } else { - ADD_ARG_LIT("-parallel"); + virCommandAddArg(cmd, "-parallel"); if (!(devstr = qemuBuildChrArgStr(parallel, NULL))) goto error; - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); } } } @@ -4977,24 +4916,26 @@ int qemudBuildCommandLine(virConnectPtr conn, goto error; } - ADD_ARG_LIT("-chardev"); + virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(channel))) goto error; - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); char *addr = virSocketFormatAddr(channel->target.addr); if (!addr) goto error; int port = virSocketGetPort(channel->target.addr); - ADD_ARG_LIT("-netdev"); + virCommandAddArg(cmd, "-netdev"); if (virAsprintf(&devstr, "user,guestfwd=tcp:%s:%i,chardev=%s,id=user-%s", addr, port, channel->info.alias, channel->info.alias) < 0) { VIR_FREE(addr); goto no_memory; } VIR_FREE(addr); - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); break; case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: @@ -5004,15 +4945,17 @@ int qemudBuildCommandLine(virConnectPtr conn, goto error; } - ADD_ARG_LIT("-chardev"); + virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(channel))) goto error; - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); - ADD_ARG_LIT("-device"); + virCommandAddArg(cmd, "-device"); if (!(devstr = qemuBuildVirtioSerialPortDevStr(channel))) goto error; - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); break; } } @@ -5030,15 +4973,17 @@ int qemudBuildCommandLine(virConnectPtr conn, goto error; } - ADD_ARG_LIT("-chardev"); + virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(console))) goto error; - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); - ADD_ARG_LIT("-device"); + virCommandAddArg(cmd, "-device"); if (!(devstr = qemuBuildVirtioSerialPortDevStr(console))) goto error; - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); break; case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL: @@ -5052,20 +4997,21 @@ int qemudBuildCommandLine(virConnectPtr conn, } } - ADD_ARG_LIT("-usb"); + virCommandAddArg(cmd, "-usb"); for (i = 0 ; i < def->ninputs ; i++) { virDomainInputDefPtr input = def->inputs[i]; if (input->bus == VIR_DOMAIN_INPUT_BUS_USB) { if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { char *optstr; - ADD_ARG_LIT("-device"); + virCommandAddArg(cmd, "-device"); if (!(optstr = qemuBuildUSBInputDevStr(input))) goto error; - ADD_ARG(optstr); + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); } else { - ADD_ARG_LIT("-usbdevice"); - ADD_ARG_LIT(input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ? "mouse" : "tablet"); + virCommandAddArg(cmd, "-usbdevice"); + virCommandAddArg(cmd, input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ? "mouse" : "tablet"); } } } @@ -5109,7 +5055,7 @@ int qemudBuildCommandLine(virConnectPtr conn, virBufferAddLit(&opt, ",sasl"); if (driver->vncSASLdir) - ADD_ENV_PAIR("SASL_CONF_DIR", driver->vncSASLdir); + virCommandAddEnvPair(cmd, "SASL_CONF_DIR", driver->vncSASLdir); /* TODO: Support ACLs later */ } @@ -5124,11 +5070,12 @@ int qemudBuildCommandLine(virConnectPtr conn, optstr = virBufferContentAndReset(&opt); - ADD_ARG_LIT("-vnc"); - ADD_ARG(optstr); + virCommandAddArg(cmd, "-vnc"); + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); if (def->graphics[0]->data.vnc.keymap) { - ADD_ARG_LIT("-k"); - ADD_ARG_LIT(def->graphics[0]->data.vnc.keymap); + virCommandAddArg(cmd, "-k"); + virCommandAddArg(cmd, def->graphics[0]->data.vnc.keymap); } /* Unless user requested it, set the audio backend to none, to @@ -5136,45 +5083,34 @@ int qemudBuildCommandLine(virConnectPtr conn, * security issues and might not work when using VNC. */ if (driver->vncAllowHostAudio) { - ADD_ENV_COPY("QEMU_AUDIO_DRV"); + virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV"); } else { - ADD_ENV_LIT("QEMU_AUDIO_DRV=none"); + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); } } else if ((def->ngraphics == 1) && def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { - char *xauth = NULL; - char *display = NULL; + if (def->graphics[0]->data.sdl.xauth) + virCommandAddEnvPair(cmd, "XAUTHORITY", + def->graphics[0]->data.sdl.xauth); + if (def->graphics[0]->data.sdl.display) + virCommandAddEnvPair(cmd, "DISPLAY", + def->graphics[0]->data.sdl.display); - if (def->graphics[0]->data.sdl.xauth && - virAsprintf(&xauth, "XAUTHORITY=%s", - def->graphics[0]->data.sdl.xauth) < 0) - goto no_memory; - if (def->graphics[0]->data.sdl.display && - virAsprintf(&display, "DISPLAY=%s", - def->graphics[0]->data.sdl.display) < 0) { - VIR_FREE(xauth); - goto no_memory; - } - - if (xauth) - ADD_ENV(xauth); - if (display) - ADD_ENV(display); if (def->graphics[0]->data.sdl.fullscreen) - ADD_ARG_LIT("-full-screen"); + virCommandAddArg(cmd, "-full-screen"); /* If using SDL for video, then we should just let it * use QEMU's host audio drivers, possibly SDL too * User can set these two before starting libvirtd */ - ADD_ENV_COPY("QEMU_AUDIO_DRV"); - ADD_ENV_COPY("SDL_AUDIODRIVER"); + virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV"); + virCommandAddEnvPass(cmd, "SDL_AUDIODRIVER"); /* New QEMU has this flag to let us explicitly ask for * SDL graphics. This is better than relying on the * default, since the default changes :-( */ if (qemuCmdFlags & QEMUD_CMD_FLAG_SDL) - ADD_ARG_LIT("-sdl"); + virCommandAddArg(cmd, "-sdl"); } else if ((def->ngraphics == 1) && def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { @@ -5227,16 +5163,17 @@ int qemudBuildCommandLine(virConnectPtr conn, optstr = virBufferContentAndReset(&opt); - ADD_ARG_LIT("-spice"); - ADD_ARG(optstr); + virCommandAddArg(cmd, "-spice"); + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); if (def->graphics[0]->data.spice.keymap) { - ADD_ARG_LIT("-k"); - ADD_ARG_LIT(def->graphics[0]->data.spice.keymap); + virCommandAddArg(cmd, "-k"); + virCommandAddArg(cmd, def->graphics[0]->data.spice.keymap); } /* SPICE includes native support for tunnelling audio, so we * set the audio backend to point at SPICE's own driver */ - ADD_ENV_LIT("QEMU_AUDIO_DRV=spice"); + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice"); } else if ((def->ngraphics == 1)) { qemuReportError(VIR_ERR_INTERNAL_ERROR, @@ -5265,18 +5202,18 @@ int qemudBuildCommandLine(virConnectPtr conn, goto error; } - ADD_ARG_LIT("-vga"); - ADD_ARG_LIT(vgastr); + virCommandAddArg(cmd, "-vga"); + virCommandAddArg(cmd, vgastr); } } else { switch (def->videos[0]->type) { case VIR_DOMAIN_VIDEO_TYPE_VGA: - ADD_ARG_LIT("-std-vga"); + virCommandAddArg(cmd, "-std-vga"); break; case VIR_DOMAIN_VIDEO_TYPE_VMVGA: - ADD_ARG_LIT("-vmwarevga"); + virCommandAddArg(cmd, "-vmwarevga"); break; case VIR_DOMAIN_VIDEO_TYPE_XEN: @@ -5303,12 +5240,13 @@ int qemudBuildCommandLine(virConnectPtr conn, goto error; } - ADD_ARG_LIT("-device"); + virCommandAddArg(cmd, "-device"); if (!(str = qemuBuildVideoDevStr(def->videos[i]))) goto error; - ADD_ARG(str); + virCommandAddArg(cmd, str); + VIR_FREE(str); } } else { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -5321,8 +5259,8 @@ int qemudBuildCommandLine(virConnectPtr conn, /* If we have -device, then we set -nodefault already */ if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) && (qemuCmdFlags & QEMUD_CMD_FLAG_VGA)) { - ADD_ARG_LIT("-vga"); - ADD_ARG_LIT("none"); + virCommandAddArg(cmd, "-vga"); + virCommandAddArg(cmd, "none"); } } @@ -5337,15 +5275,16 @@ int qemudBuildCommandLine(virConnectPtr conn, * we don't need to set any PCI address on it, so we don't * mind too much */ if (sound->model == VIR_DOMAIN_SOUND_MODEL_PCSPK) { - ADD_ARG_LIT("-soundhw"); - ADD_ARG_LIT("pcspk"); + virCommandAddArg(cmd, "-soundhw"); + virCommandAddArg(cmd, "pcspk"); } else { - ADD_ARG_LIT("-device"); + virCommandAddArg(cmd, "-device"); if (!(str = qemuBuildSoundDevStr(sound))) goto error; - ADD_ARG(str); + virCommandAddArg(cmd, str); + VIR_FREE(str); } } } else { @@ -5368,8 +5307,9 @@ int qemudBuildCommandLine(virConnectPtr conn, if (i < (def->nsounds - 1)) strncat(modstr, ",", size--); } - ADD_ARG_LIT("-soundhw"); - ADD_ARG(modstr); + virCommandAddArg(cmd, "-soundhw"); + virCommandAddArg(cmd, modstr); + VIR_FREE(modstr); } } @@ -5379,13 +5319,13 @@ int qemudBuildCommandLine(virConnectPtr conn, char *optstr; if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { - ADD_ARG_LIT("-device"); + virCommandAddArg(cmd, "-device"); optstr = qemuBuildWatchdogDevStr(watchdog); if (!optstr) goto error; } else { - ADD_ARG_LIT("-watchdog"); + virCommandAddArg(cmd, "-watchdog"); const char *model = virDomainWatchdogModelTypeToString(watchdog->model); if (!model) { @@ -5397,7 +5337,8 @@ int qemudBuildCommandLine(virConnectPtr conn, if (!(optstr = strdup(model))) goto no_memory; } - ADD_ARG(optstr); + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); const char *action = virDomainWatchdogActionTypeToString(watchdog->action); if (!action) { @@ -5405,8 +5346,8 @@ int qemudBuildCommandLine(virConnectPtr conn, "%s", _("invalid watchdog action")); goto error; } - ADD_ARG_LIT("-watchdog-action"); - ADD_ARG_LIT(action); + virCommandAddArg(cmd, "-watchdog-action"); + virCommandAddArg(cmd, action); } /* Add host passthrough hardware */ @@ -5419,15 +5360,17 @@ int qemudBuildCommandLine(virConnectPtr conn, hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { - ADD_ARG_LIT("-device"); + virCommandAddArg(cmd, "-device"); if (!(devstr = qemuBuildUSBHostdevDevStr(hostdev))) goto error; - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); } else { - ADD_ARG_LIT("-usbdevice"); + virCommandAddArg(cmd, "-usbdevice"); if (!(devstr = qemuBuildUSBHostdevUsbDevStr(hostdev))) goto error; - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); } } @@ -5446,26 +5389,22 @@ int qemudBuildCommandLine(virConnectPtr conn, goto no_memory; } - if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) { - VIR_FREE(configfd_name); - VIR_FORCE_CLOSE(configfd); - goto no_memory; - } - - (*vmfds)[(*nvmfds)++] = configfd; + virCommandPreserveFD(cmd, configfd); } } - ADD_ARG_LIT("-device"); + virCommandAddArg(cmd, "-device"); devstr = qemuBuildPCIHostdevDevStr(hostdev, configfd_name); VIR_FREE(configfd_name); if (!devstr) goto error; - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); } else if (qemuCmdFlags & QEMUD_CMD_FLAG_PCIDEVICE) { - ADD_ARG_LIT("-pcidevice"); + virCommandAddArg(cmd, "-pcidevice"); if (!(devstr = qemuBuildPCIHostdevPCIDevStr(hostdev))) goto error; - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); } else { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("PCI device assignment is not supported by this version of qemu")); @@ -5475,8 +5414,8 @@ int qemudBuildCommandLine(virConnectPtr conn, } if (migrateFrom) { - ADD_ARG_LIT("-incoming"); - ADD_ARG_LIT(migrateFrom); + virCommandAddArg(cmd, "-incoming"); + virCommandAddArg(cmd, migrateFrom); } /* QEMU changed its default behavior to not include the virtio balloon @@ -5495,76 +5434,47 @@ int qemudBuildCommandLine(virConnectPtr conn, } if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { char *optstr; - ADD_ARG_LIT("-device"); + virCommandAddArg(cmd, "-device"); optstr = qemuBuildMemballoonDevStr(def->memballoon); if (!optstr) goto error; - ADD_ARG(optstr); + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); } else if (qemuCmdFlags & QEMUD_CMD_FLAG_BALLOON) { - ADD_ARG_LIT("-balloon"); - ADD_ARG_LIT("virtio"); + virCommandAddArg(cmd, "-balloon"); + virCommandAddArg(cmd, "virtio"); } } if (current_snapshot && current_snapshot->def->active) { - ADD_ARG_LIT("-loadvm"); - ADD_ARG_LIT(current_snapshot->def->name); + virCommandAddArg(cmd, "-loadvm"); + virCommandAddArg(cmd, current_snapshot->def->name); } if (def->namespaceData) { - qemuDomainCmdlineDefPtr cmd; - - cmd = def->namespaceData; - for (i = 0; i < cmd->num_args; i++) - ADD_ARG_LIT(cmd->args[i]); - for (i = 0; i < cmd->num_env; i++) { - if (cmd->env_value[i]) - ADD_ENV_PAIR(cmd->env_name[i], cmd->env_value[i]); + qemuDomainCmdlineDefPtr qemucmd; + + qemucmd = def->namespaceData; + for (i = 0; i < qemucmd->num_args; i++) + virCommandAddArg(cmd, qemucmd->args[i]); + for (i = 0; i < qemucmd->num_env; i++) { + if (qemucmd->env_value[i]) + virCommandAddEnvPair(cmd, qemucmd->env_name[i], qemucmd->env_value[i]); else - ADD_ENV_PAIR(cmd->env_name[i], ""); + virCommandAddEnvPair(cmd, qemucmd->env_name[i], ""); } } - ADD_ARG(NULL); - ADD_ENV(NULL); - - *retargv = qargv; - *retenv = qenv; - return 0; + return cmd; no_memory: virReportOOMError(); error: for (i = 0; i <= last_good_net; i++) virDomainConfNWFilterTeardown(def->nets[i]); - if (vmfds && - *vmfds) { - for (i = 0; i < *nvmfds; i++) - VIR_FORCE_CLOSE((*vmfds)[i]); - VIR_FREE(*vmfds); - *nvmfds = 0; - } - if (qargv) { - for (i = 0 ; i < qargc ; i++) - VIR_FREE((qargv)[i]); - VIR_FREE(qargv); - } - if (qenv) { - for (i = 0 ; i < qenvc ; i++) - VIR_FREE((qenv)[i]); - VIR_FREE(qenv); - } - return -1; - -#undef ADD_ARG -#undef ADD_ARG_LIT -#undef ADD_ARG_SPACE -#undef ADD_USBDISK -#undef ADD_ENV -#undef ADD_ENV_COPY -#undef ADD_ENV_LIT -#undef ADD_ENV_SPACE + virCommandFree(cmd); + return NULL; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 3789ed2..b006e75 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -41,6 +41,7 @@ # include "driver.h" # include "bitmap.h" # include "locking/lock_manager.h" +# include "command.h" # define qemudDebug(fmt, ...) do {} while(0) @@ -230,18 +231,14 @@ int qemudParseHelpStr (const char *qemu, unsigned int *is_kvm, unsigned int *kvm_version); -int qemudBuildCommandLine (virConnectPtr conn, - struct qemud_driver *driver, - virDomainDefPtr def, - virDomainChrDefPtr monitor_chr, - int monitor_json, - unsigned long long qemuCmdFlags, - const char ***retargv, - const char ***retenv, - int **vmfds, - int *nvmfds, - const char *migrateFrom, - virDomainSnapshotObjPtr current_snapshot) +virCommandPtr qemudBuildCommandLine(virConnectPtr conn, + struct qemud_driver *driver, + virDomainDefPtr def, + virDomainChrDefPtr monitor_chr, + int monitor_json, + unsigned long long qemuCmdFlags, + const char *migrateFrom, + virDomainSnapshotObjPtr current_snapshot) ATTRIBUTE_NONNULL(1); /* With vlan == -1, use netdev syntax, else old hostnet */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9dcf02d..0b0b6fe 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3017,14 +3017,6 @@ qemuAssignPCIAddresses(virDomainDefPtr def) int ret = -1; unsigned long long qemuCmdFlags = 0; qemuDomainPCIAddressSetPtr addrs = NULL; - struct stat sb; - - if (stat(def->emulator, &sb) < 0) { - virReportSystemError(errno, - _("Cannot find QEMU binary %s"), - def->emulator); - goto cleanup; - } if (qemudExtractVersionInfo(def->emulator, NULL, @@ -3881,16 +3873,8 @@ static int qemudStartVMDaemon(virConnectPtr conn, bool start_paused, int stdin_fd, const char *stdin_path) { - const char **argv = NULL, **tmp; - const char **progenv = NULL; - int i, ret, runflags; - struct stat sb; - int *vmfds = NULL; - int nvmfds = 0; + int ret; unsigned long long qemuCmdFlags; - fd_set keepfd; - const char *emulator; - pid_t child; int pos = -1; char ebuf[1024]; char *pidfile = NULL; @@ -3898,6 +3882,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, char *timestamp; qemuDomainObjPrivatePtr priv = vm->privateData; virDomainLockPtr lock = NULL; + virCommandPtr cmd = NULL; struct qemudHookData hookData; hookData.conn = conn; @@ -3905,8 +3890,6 @@ static int qemudStartVMDaemon(virConnectPtr conn, hookData.driver = driver; hookData.lockState = NULL; /* XXX add lock state from migration source ! */ - FD_ZERO(&keepfd); - DEBUG0("Beginning VM startup process"); if (virDomainObjIsActive(vm)) { @@ -3987,21 +3970,8 @@ static int qemudStartVMDaemon(virConnectPtr conn, if ((logfile = qemudLogFD(driver, vm->def->name)) < 0) goto cleanup; - emulator = vm->def->emulator; - - /* Make sure the binary we are about to try exec'ing exists. - * Technically we could catch the exec() failure, but that's - * in a sub-process so its hard to feed back a useful error - */ - if (stat(emulator, &sb) < 0) { - virReportSystemError(errno, - _("Cannot find QEMU binary %s"), - emulator); - goto cleanup; - } - DEBUG0("Determing emulator version"); - if (qemudExtractVersionInfo(emulator, + if (qemudExtractVersionInfo(vm->def->emulator, NULL, &qemuCmdFlags) < 0) goto cleanup; @@ -4070,10 +4040,10 @@ static int qemudStartVMDaemon(virConnectPtr conn, DEBUG0("Building emulator command line"); vm->def->id = driver->nextvmid++; - if (qemudBuildCommandLine(conn, driver, vm->def, priv->monConfig, - priv->monJSON, qemuCmdFlags, &argv, &progenv, - &vmfds, &nvmfds, migrateFrom, - vm->current_snapshot) < 0) + if (!(cmd = qemudBuildCommandLine(conn, driver, vm->def, priv->monConfig, + priv->monJSON, qemuCmdFlags, + migrateFrom, + vm->current_snapshot))) goto cleanup; if (qemuDomainSnapshotSetInactive(vm, driver->snapshotDir) < 0) @@ -4108,70 +4078,46 @@ static int qemudStartVMDaemon(virConnectPtr conn, VIR_FREE(timestamp); } - tmp = progenv; - - while (*tmp) { - if (safewrite(logfile, *tmp, strlen(*tmp)) < 0) - VIR_WARN("Unable to write envv to logfile: %s", - virStrerror(errno, ebuf, sizeof ebuf)); - if (safewrite(logfile, " ", 1) < 0) - VIR_WARN("Unable to write envv to logfile: %s", - virStrerror(errno, ebuf, sizeof ebuf)); - tmp++; - } - tmp = argv; - while (*tmp) { - if (safewrite(logfile, *tmp, strlen(*tmp)) < 0) - VIR_WARN("Unable to write argv to logfile: %s", - virStrerror(errno, ebuf, sizeof ebuf)); - if (safewrite(logfile, " ", 1) < 0) - VIR_WARN("Unable to write argv to logfile: %s", - virStrerror(errno, ebuf, sizeof ebuf)); - tmp++; - } - if (safewrite(logfile, "\n", 1) < 0) - VIR_WARN("Unable to write argv to logfile: %s", - virStrerror(errno, ebuf, sizeof ebuf)); + virCommandWriteArgLog(cmd, logfile); if ((pos = lseek(logfile, 0, SEEK_END)) < 0) VIR_WARN("Unable to seek to end of logfile: %s", virStrerror(errno, ebuf, sizeof ebuf)); - for (i = 0 ; i < nvmfds ; i++) - FD_SET(vmfds[i], &keepfd); - VIR_DEBUG("Clear emulator capabilities: %d", driver->clearEmulatorCapabilities); - runflags = VIR_EXEC_NONBLOCK; - if (driver->clearEmulatorCapabilities) { - runflags |= VIR_EXEC_CLEAR_CAPS; - } - - VIR_WARN("Executing %s", argv[0]); - ret = virExecDaemonize(argv, progenv, &keepfd, &child, - stdin_fd, &logfile, &logfile, - runflags, - qemudSecurityHook, &hookData, - pidfile); - VIR_WARN("Executing done %s", argv[0]); + /** XXXX runflags = VIR_EXEC_NONBLOCK; */ + if (driver->clearEmulatorCapabilities) + virCommandClearCaps(cmd); + + VIR_WARN("Executing %s", vm->def->emulator); + virCommandSetPreExecHook(cmd, qemudSecurityHook, &hookData); + virCommandSetInputFD(cmd, stdin_fd); + virCommandSetOutputFD(cmd, &logfile); + virCommandSetErrorFD(cmd, &logfile); + virCommandSetPidFile(cmd, pidfile); + virCommandDaemonize(cmd); + + ret = virCommandRun(cmd, NULL); + VIR_WARN("Executing done %s", vm->def->emulator); VIR_FREE(pidfile); /* XXX this is bollocks. Need a sync point */ sleep(5); - VIR_WARN("Locking %s", argv[0]); + VIR_WARN("Locking %s", vm->def->emulator); if (!(lock = virDomainLockForStartup(driver->contentLockManager, driver->metadataLockManager, NULL, /* XXX lock state */ vm))) goto cleanup; - VIR_WARN("Labelling %s", argv[0]); + VIR_WARN("Labelling %s", vm->def->emulator); DEBUG0("Setting domain security labels"); if (virSecurityManagerSetAllLabel(driver->securityManager, vm, stdin_path) < 0) goto cleanup; - VIR_WARN("All done %s", argv[0]); + VIR_WARN("All done %s", vm->def->emulator); /* wait for qemu process to to show up */ @@ -4181,6 +4127,11 @@ static int qemudStartVMDaemon(virConnectPtr conn, _("Domain %s didn't show up\n"), vm->def->name); ret = -1; } +#if 0 + /* + * XXX this is bogus. It isn't safe to set vm->pid = chidl + * because child no longer exists. + */ } else if (ret == -2) { /* The virExec process that launches the daemon failed. Pending on * when it failed (we can't determine for sure), there may be @@ -4191,30 +4142,16 @@ static int qemudStartVMDaemon(virConnectPtr conn, */ vm->pid = child; ret = 0; +#endif } if (migrateFrom) start_paused = true; vm->state = start_paused ? VIR_DOMAIN_PAUSED : VIR_DOMAIN_RUNNING; - for (i = 0 ; argv[i] ; i++) - VIR_FREE(argv[i]); - VIR_FREE(argv); - - for (i = 0 ; progenv[i] ; i++) - VIR_FREE(progenv[i]); - VIR_FREE(progenv); - if (ret == -1) /* The VM failed to start; tear filters before taps */ virDomainConfVMNWFilterTeardown(vm); - if (vmfds) { - for (i = 0 ; i < nvmfds ; i++) { - VIR_FORCE_CLOSE(vmfds[i]); - } - VIR_FREE(vmfds); - } - if (ret == -1) /* The VM failed to start */ goto cleanup; @@ -4264,6 +4201,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, goto cleanup; virDomainLockReleaseAndFree(lock); + virCommandFree(cmd); VIR_FORCE_CLOSE(logfile); @@ -4274,14 +4212,15 @@ cleanup: * that will re-aquire the lock in order to perform relabelling */ virDomainLockReleaseAndFree(lock); + virCommandFree(cmd); + + VIR_FORCE_CLOSE(logfile); /* We jump here if we failed to start the VM for any reason, or * if we failed to initialize the now running VM. kill it off and * pretend we never started it */ qemudShutdownVMDaemon(driver, vm, 0); - VIR_FORCE_CLOSE(logfile); - return -1; } @@ -7312,13 +7251,8 @@ static char *qemuDomainXMLToNative(virConnectPtr conn, struct qemud_driver *driver = conn->privateData; virDomainDefPtr def = NULL; virDomainChrDef monConfig; - const char *emulator; unsigned long long qemuCmdFlags; - struct stat sb; - const char **retargv = NULL; - const char **retenv = NULL; - const char **tmp; - virBuffer buf = VIR_BUFFER_INITIALIZER; + virCommandPtr cmd = NULL; char *ret = NULL; int i; @@ -7368,69 +7302,24 @@ static char *qemuDomainXMLToNative(virConnectPtr conn, def->graphics[i]->data.vnc.autoport) def->graphics[i]->data.vnc.port = 5900; } - emulator = def->emulator; - - /* Make sure the binary we are about to try exec'ing exists. - * Technically we could catch the exec() failure, but that's - * in a sub-process so its hard to feed back a useful error - */ - if (stat(emulator, &sb) < 0) { - virReportSystemError(errno, - _("Cannot find QEMU binary %s"), - emulator); - goto cleanup; - } - if (qemudExtractVersionInfo(emulator, + if (qemudExtractVersionInfo(def->emulator, NULL, - &qemuCmdFlags) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot determine QEMU argv syntax %s"), - emulator); + &qemuCmdFlags) < 0) goto cleanup; - } if (qemuPrepareMonitorChr(driver, &monConfig, def->name) < 0) goto cleanup; - if (qemudBuildCommandLine(conn, driver, def, - &monConfig, 0, qemuCmdFlags, - &retargv, &retenv, - NULL, NULL, /* Don't want it to create TAP devices */ - NULL, NULL) < 0) { - goto cleanup; - } - - tmp = retenv; - while (*tmp) { - virBufferAdd(&buf, *tmp, strlen(*tmp)); - virBufferAddLit(&buf, " "); - tmp++; - } - tmp = retargv; - while (*tmp) { - virBufferAdd(&buf, *tmp, strlen(*tmp)); - virBufferAddLit(&buf, " "); - tmp++; - } - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); + if (!(cmd = qemudBuildCommandLine(conn, driver, def, + &monConfig, 0, qemuCmdFlags, + NULL, NULL))) goto cleanup; - } - ret = virBufferContentAndReset(&buf); + ret = virCommandToString(cmd); cleanup: qemuDriverUnlock(driver); - for (tmp = retargv ; tmp && *tmp ; tmp++) - VIR_FREE(*tmp); - VIR_FREE(retargv); - - for (tmp = retenv ; tmp && *tmp ; tmp++) - VIR_FREE(*tmp); - VIR_FREE(retenv); virDomainDefFree(def); return ret; -- 1.7.2.3

On 11/22/2010 11:09 AM, Daniel P. Berrange wrote: [reviving this thread a bit] Ah - I see you stumbled on the same problem that has already been fixed. You won't need this hunk :) Hmm, I already implemented a static function virCommandHook() for managing cwd swapping; this should be merged into that function. trailing space Looks correct for the hook. if (!cmd || cmd->has_error) return; Oops; forgot to set cmd->has_error in this case. And this is the first instance of setting cmd->has_error to an errno other than ENOMEM; we'll have to make sure virCommandRun() does the right thing in this case. Also, we should have a sanity check that use of RequireHandshake entails that virCommandRunAsync (and not virCommandRun) is called, so that we don't deadlock. Can RequireHandshake and Daemonize be mixed, or should our sanity checks declare them as incompatible? Would these be better as virCommandTransferFD()? Needs error checking that the command has been started but not yet waited on (that is, cmd->pid should be non-negative). Likewise. Should this also check that handshakewait has been called first? Should these last two be marked ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK? Do you want me to take over modernizing this patch, and updating tests/commandtest.c to exercise it, since I've been doing other virCommand code? -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 11/22/2010 11:09 AM, Daniel P. Berrange wrote: Oh, I see - you want to daemonize, require handshake, AND do a blocking virCommandRun. I see now - __virExec does two forks when daemonize is requested, and only runs the callback hook in the second (daemon) child, rather than in the (short-lived) intermediary. That answers some of my questions in the previous patch. But it still probably requires virCommand to do some sanity checking that HandshakeWait is called after the child has been spawned. s/hanshake/handshake/ Definitely an improvement. -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org