Markus Groà wrote:
> ---
> src/libxl/libxl_driver.c | 353 ++++++++++++++++++++++++++++++++++++----------
> 1 files changed, 276 insertions(+), 77 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 61c3494..9bcc3b9 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -66,7 +66,6 @@ static int
> libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> bool start_paused, int restore_fd);
>
> -
> /* Function definitions */
> static void
> libxlDriverLock(libxlDriverPrivatePtr driver)
> @@ -219,6 +218,87 @@ libxlDoNodeGetInfo(libxlDriverPrivatePtr driver, virNodeInfoPtr info)
> return 0;
> }
>
> +static char *
> +libxlDomainManagedSavePath(libxlDriverPrivatePtr driver, virDomainObjPtr vm) {
> + char *ret;
> +
> + if (virAsprintf(&ret, "%s/%s.save", driver->saveDir, vm->def->name) < 0) {
> + virReportOOMError();
> + return NULL;
> + }
> +
> + return ret;
> +}
> +
> +/* This internal function expects the driver lock to already be held on
> + * entry. */
> +static int ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
> +libxlSaveImageOpen(libxlDriverPrivatePtr driver, const char *from,
> + virDomainDefPtr *ret_def, libxlSavefileHeaderPtr ret_hdr)
> +{
> + int fd;
> + virDomainDefPtr def = NULL;
> + libxlSavefileHeader hdr;
> + char *xml = NULL;
> +
> + if ((fd = virFileOpenAs(from, O_RDONLY, 0, getuid(), getgid(), 0)) < 0) {
> + libxlError(VIR_ERR_OPERATION_FAILED,
> + "%s", _("cannot read domain image"));
> + goto error;
> + }
> +
> + if (saferead(fd, &hdr, sizeof(hdr)) != sizeof(hdr)) {
> + libxlError(VIR_ERR_OPERATION_FAILED,
> + "%s", _("failed to read libxl header"));
> + goto error;
> + }
> +
> + if (memcmp(hdr.magic, LIBXL_SAVE_MAGIC, sizeof(hdr.magic))) {
> + libxlError(VIR_ERR_INVALID_ARG, "%s", _("image magic is incorrect"));
> + goto error;
> + }
> +
> + if (hdr.version > LIBXL_SAVE_VERSION) {
> + libxlError(VIR_ERR_OPERATION_FAILED,
> + _("image version is not supported (%d > %d)"),
> + hdr.version, LIBXL_SAVE_VERSION);
> + goto error;
> + }
> +
> + if (hdr.xmlLen <= 0) {
> + libxlError(VIR_ERR_OPERATION_FAILED,
> + _("invalid XML length: %d"), hdr.xmlLen);
> + goto error;
> + }
> +
> + if (VIR_ALLOC_N(xml, hdr.xmlLen) < 0) {
> + virReportOOMError();
> + goto error;
> + }
> +
> + if (saferead(fd, xml, hdr.xmlLen) != hdr.xmlLen) {
> + libxlError(VIR_ERR_OPERATION_FAILED, "%s", _("failed to read XML"));
> + goto error;
> + }
> +
> + if (!(def = virDomainDefParseString(driver->caps, xml,
> + VIR_DOMAIN_XML_INACTIVE)))
> + goto error;
> +
> + VIR_FREE(xml);
> +
> + *ret_def = def;
> + *ret_hdr = hdr;
> +
> + return fd;
> +
> +error:
> + VIR_FREE(xml);
> + virDomainDefFree(def);
> + VIR_FORCE_CLOSE(fd);
> + return -1;
> +}
> +
> /*
> * Cleanup function for domain that has reached shutoff state.
> *
> @@ -546,17 +626,53 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> bool start_paused, int restore_fd)
> {
> libxl_domain_config d_config;
> - virDomainDefPtr def = vm->def;
> + virDomainDefPtr def = NULL;
> virDomainEventPtr event = NULL;
> + libxlSavefileHeader hdr;
> int ret;
> uint32_t domid = 0;
> char *dom_xml = NULL;
> + char *managed_save = NULL;
> pid_t child_console_pid = -1;
> libxlDomainObjPrivatePtr priv = vm->privateData;
>
> + /* If there is a managed saved state restore it instead of starting
> + * from scratch. The old state is removed once the restoring succeeded. */
> + if (restore_fd < 0) {
> + managed_save = libxlDomainManagedSavePath(driver, vm);
> + if (managed_save == NULL)
> + goto error;
> +
> + if (virFileExists(managed_save)) {
> +
> + restore_fd = libxlSaveImageOpen(driver, managed_save, &def, &hdr);
> + if (restore_fd < 0)
> + goto error;
> +
> + if (STRNEQ(vm->def->name, def->name) ||
> + memcmp(vm->def->uuid, def->uuid, VIR_UUID_BUFLEN)) {
> + char vm_uuidstr[VIR_UUID_STRING_BUFLEN];
> + char def_uuidstr[VIR_UUID_STRING_BUFLEN];
> + virUUIDFormat(vm->def->uuid, vm_uuidstr);
> + virUUIDFormat(def->uuid, def_uuidstr);
> + libxlError(VIR_ERR_OPERATION_FAILED,
> + _("cannot restore domain '%s' uuid %s from a file"
> + " which belongs to domain '%s' uuid %s"),
> + vm->def->name, vm_uuidstr, def->name, def_uuidstr);
> + goto error;
> + }
> +
> + virDomainObjAssignDef(vm, def, true);
> + def = NULL;
> +
> + if (unlink(managed_save) < 0)
> + VIR_WARN("Failed to remove the managed state %s", managed_save);
> + }
> + }
> +
>
On success of this function, managed_save and restore_fd will leak.
> memset(&d_config, 0, sizeof(d_config));
>
> - if (libxlBuildDomainConfig(driver, def, &d_config) < 0 )
> + if (libxlBuildDomainConfig(driver, vm->def, &d_config) < 0 )
> return -1;
>
> if (libxlFreeMem(priv, &d_config) < 0) {
> @@ -586,8 +702,8 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> goto error;
> }
>
> - def->id = domid;
> - if ((dom_xml = virDomainDefFormat(def, 0)) == NULL)
> + vm->def->id = domid;
> + if ((dom_xml = virDomainDefFormat(vm->def, 0)) == NULL)
> goto error;
>
> if (libxl_userdata_store(&priv->ctx, domid, "libvirt-xml",
> @@ -627,11 +743,14 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> error:
> if (domid > 0) {
> libxl_domain_destroy(&priv->ctx, domid, 0);
> - def->id = -1;
> + vm->def->id = -1;
> virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_FAILED);
> }
> libxl_domain_config_destroy(&d_config);
> VIR_FREE(dom_xml);
> + VIR_FREE(managed_save);
> + virDomainDefFree(def);
> + VIR_FORCE_CLOSE(restore_fd);
>
This would close a caller-provided fd. I think it is better for callers
that have opened an fd to also close it.
I'll send a patch that addresses these minor issues. Otherwise looks
good and no problems noted in my testing.
Regards,
Jim
> return -1;
> }
>
> @@ -1692,12 +1811,13 @@ libxlDomainGetState(virDomainPtr dom,
> return ret;
> }
>
> +/* This internal function expects the driver lock to already be held on
> + * entry and the vm must be active. */
> static int
> -libxlDomainSave(virDomainPtr dom, const char *to)
> +libxlDoDomainSave(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> + const char *to)
> {
> - libxlDriverPrivatePtr driver = dom->conn->privateData;
> - virDomainObjPtr vm;
> - libxlDomainObjPrivatePtr priv;
> + libxlDomainObjPrivatePtr priv = vm->privateData;
> libxlSavefileHeader hdr;
> virDomainEventPtr event = NULL;
> char *xml = NULL;
> @@ -1705,28 +1825,10 @@ libxlDomainSave(virDomainPtr dom, const char *to)
> int fd;
> int ret = -1;
>
> - libxlDriverLock(driver);
> - vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> -
> - if (!vm) {
> - char uuidstr[VIR_UUID_STRING_BUFLEN];
> - virUUIDFormat(dom->uuid, uuidstr);
> - libxlError(VIR_ERR_NO_DOMAIN,
> - _("No domain with matching uuid '%s'"), uuidstr);
> - goto cleanup;
> - }
> -
> - if (!virDomainObjIsActive(vm)) {
> - libxlError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running"));
> - goto cleanup;
> - }
> -
> - priv = vm->privateData;
> -
> if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) {
> libxlError(VIR_ERR_OPERATION_INVALID,
> _("Domain '%d' has to be running because libxenlight will"
> - " suspend it"), dom->id);
> + " suspend it"), vm->def->id);
> goto cleanup;
> }
>
> @@ -1758,10 +1860,10 @@ libxlDomainSave(virDomainPtr dom, const char *to)
> goto cleanup;
> }
>
> - if (libxl_domain_suspend(&priv->ctx, NULL, dom->id, fd) != 0) {
> + if (libxl_domain_suspend(&priv->ctx, NULL, vm->def->id, fd) != 0) {
> libxlError(VIR_ERR_INTERNAL_ERROR,
> _("Failed to save domain '%d' with libxenlight"),
> - dom->id);
> + vm->def->id);
> goto cleanup;
> }
>
> @@ -1770,7 +1872,7 @@ libxlDomainSave(virDomainPtr dom, const char *to)
>
> if (libxlVmReap(driver, vm, 1, VIR_DOMAIN_SHUTOFF_SAVED) != 0) {
> libxlError(VIR_ERR_INTERNAL_ERROR,
> - _("Failed to destroy domain '%d'"), dom->id);
> + _("Failed to destroy domain '%d'"), vm->def->id);
> goto cleanup;
> }
>
> @@ -1785,69 +1887,57 @@ cleanup:
> VIR_FREE(xml);
> if (VIR_CLOSE(fd) < 0)
> virReportSystemError(errno, "%s", _("cannot close file"));
> - if (vm)
> - virDomainObjUnlock(vm);
> if (event)
> libxlDomainEventQueue(driver, event);
> - libxlDriverUnlock(driver);
> return ret;
> }
>
> static int
> -libxlDomainRestore(virConnectPtr conn, const char *from)
> +libxlDomainSave(virDomainPtr dom, const char *to)
> {
> - libxlDriverPrivatePtr driver = conn->privateData;
> - virDomainDefPtr def = NULL;
> - virDomainObjPtr vm = NULL;
> - libxlSavefileHeader hdr;
> - char *xml = NULL;
> - int fd;
> + libxlDriverPrivatePtr driver = dom->conn->privateData;
> + virDomainObjPtr vm;
> int ret = -1;
>
> libxlDriverLock(driver);
> + vm = virDomainFindByUUID(&driver->domains, dom->uuid);
>
> - if ((fd = virFileOpenAs(from, O_RDONLY, 0, getuid(), getgid(), 0)) < 0) {
> - libxlError(VIR_ERR_OPERATION_FAILED,
> - "%s", _("cannot read domain image"));
> - goto cleanup;
> - }
> -
> - if (saferead(fd, &hdr, sizeof(hdr)) != sizeof(hdr)) {
> - libxlError(VIR_ERR_OPERATION_FAILED,
> - "%s", _("failed to read libxl header"));
> + if (!vm) {
> + char uuidstr[VIR_UUID_STRING_BUFLEN];
> + virUUIDFormat(dom->uuid, uuidstr);
> + libxlError(VIR_ERR_NO_DOMAIN,
> + _("No domain with matching uuid '%s'"), uuidstr);
> goto cleanup;
> }
>
> - if (memcmp(hdr.magic, LIBXL_SAVE_MAGIC, sizeof(hdr.magic))) {
> - libxlError(VIR_ERR_INVALID_ARG, "%s", _("image magic is incorrect"));
> + if (!virDomainObjIsActive(vm)) {
> + libxlError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running"));
> goto cleanup;
> }
>
> - if (hdr.version > LIBXL_SAVE_VERSION) {
> - libxlError(VIR_ERR_OPERATION_FAILED,
> - _("image version is not supported (%d > %d)"),
> - hdr.version, LIBXL_SAVE_VERSION);
> - goto cleanup;
> - }
> + ret = libxlDoDomainSave(driver, vm, to);
>
> - if (hdr.xmlLen <= 0) {
> - libxlError(VIR_ERR_OPERATION_FAILED,
> - _("invalid XML length: %d"), hdr.xmlLen);
> - goto cleanup;
> - }
> +cleanup:
> + if (vm)
> + virDomainObjUnlock(vm);
> + libxlDriverUnlock(driver);
> + return ret;
> +}
>
> - if (VIR_ALLOC_N(xml, hdr.xmlLen) < 0) {
> - virReportOOMError();
> - goto cleanup;
> - }
> +static int
> +libxlDomainRestore(virConnectPtr conn, const char *from)
> +{
> + libxlDriverPrivatePtr driver = conn->privateData;
> + virDomainObjPtr vm = NULL;
> + virDomainDefPtr def = NULL;
> + libxlSavefileHeader hdr;
> + int fd = -1;
> + int ret = -1;
>
> - if (saferead(fd, xml, hdr.xmlLen) != hdr.xmlLen) {
> - libxlError(VIR_ERR_OPERATION_FAILED, "%s", _("failed to read XML"));
> - goto cleanup;
> - }
> + libxlDriverLock(driver);
>
> - if (!(def = virDomainDefParseString(driver->caps, xml,
> - VIR_DOMAIN_XML_INACTIVE)))
> + fd = libxlSaveImageOpen(driver, from, &def, &hdr);
> + if (fd < 0)
> goto cleanup;
>
> if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0)
> @@ -1865,10 +1955,7 @@ libxlDomainRestore(virConnectPtr conn, const char *from)
> }
>
> cleanup:
> - VIR_FREE(xml);
> virDomainDefFree(def);
> - if (VIR_CLOSE(fd) < 0)
> - virReportSystemError(errno, "%s", _("cannot close file"));
> if (vm)
> virDomainObjUnlock(vm);
> libxlDriverUnlock(driver);
> @@ -1970,6 +2057,115 @@ cleanup:
> }
>
> static int
> +libxlDomainManagedSave(virDomainPtr dom, unsigned int flags)
> +{
> + libxlDriverPrivatePtr driver = dom->conn->privateData;
> + virDomainObjPtr vm = NULL;
> + char *name = NULL;
> + int ret = -1;
> +
> + virCheckFlags(0, -1);
> +
> + libxlDriverLock(driver);
> + vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> + if (!vm) {
> + char uuidstr[VIR_UUID_STRING_BUFLEN];
> + virUUIDFormat(dom->uuid, uuidstr);
> + libxlError(VIR_ERR_NO_DOMAIN,
> + _("No domain with matching uuid '%s'"), uuidstr);
> + goto cleanup;
> + }
> +
> + if (!virDomainObjIsActive(vm)) {
> + libxlError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running"));
> + goto cleanup;
> + }
> +
> + name = libxlDomainManagedSavePath(driver, vm);
> + if (name == NULL)
> + goto cleanup;
> +
> + VIR_INFO("Saving state to %s", name);
> +
> + ret = libxlDoDomainSave(driver, vm, name);
> +
> +cleanup:
> + if (vm)
> + virDomainObjUnlock(vm);
> + libxlDriverUnlock(driver);
> + VIR_FREE(name);
> + return ret;
> +}
> +
> +static int
> +libxlDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags)
> +{
> + libxlDriverPrivatePtr driver = dom->conn->privateData;
> + virDomainObjPtr vm = NULL;
> + int ret = -1;
> + char *name = NULL;
> +
> + virCheckFlags(0, -1);
> +
> + libxlDriverLock(driver);
> + vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> + if (!vm) {
> + char uuidstr[VIR_UUID_STRING_BUFLEN];
> + virUUIDFormat(dom->uuid, uuidstr);
> + libxlError(VIR_ERR_NO_DOMAIN,
> + _("No domain with matching uuid '%s'"), uuidstr);
> + goto cleanup;
> + }
> +
> + name = libxlDomainManagedSavePath(driver, vm);
> + if (name == NULL)
> + goto cleanup;
> +
> + ret = virFileExists(name);
> +
> +cleanup:
> + VIR_FREE(name);
> + if (vm)
> + virDomainObjUnlock(vm);
> + libxlDriverUnlock(driver);
> + return ret;
> +}
> +
> +static int
> +libxlDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
> +{
> + libxlDriverPrivatePtr driver = dom->conn->privateData;
> + virDomainObjPtr vm = NULL;
> + int ret = -1;
> + char *name = NULL;
> +
> + virCheckFlags(0, -1);
> +
> + libxlDriverLock(driver);
> + vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> + if (!vm) {
> + char uuidstr[VIR_UUID_STRING_BUFLEN];
> + virUUIDFormat(dom->uuid, uuidstr);
> + libxlError(VIR_ERR_NO_DOMAIN,
> + _("No domain with matching uuid '%s'"), uuidstr);
> + goto cleanup;
> + }
> +
> + name = libxlDomainManagedSavePath(driver, vm);
> + if (name == NULL)
> + goto cleanup;
> +
> + ret = unlink(name);
> +
> +cleanup:
> + VIR_FREE(name);
> + if (vm)
> + virDomainObjUnlock(vm);
> + libxlDriverUnlock(driver);
> + return ret;
> +}
> +
> +static int
> libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
> unsigned int flags)
> {
> @@ -3634,6 +3830,9 @@ static virDriver libxlDriver = {
> .nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
> .domainEventRegister = libxlDomainEventRegister, /* 0.9.0 */
> .domainEventDeregister = libxlDomainEventDeregister, /* 0.9.0 */
> + .domainManagedSave = libxlDomainManagedSave, /* 0.9.3 */
> + .domainHasManagedSaveImage = libxlDomainHasManagedSaveImage, /* 0.9.3 */
> + .domainManagedSaveRemove = libxlDomainManagedSaveRemove, /* 0.9.3 */
> .domainIsActive = libxlDomainIsActive, /* 0.9.0 */
> .domainIsPersistent = libxlDomainIsPersistent, /* 0.9.0 */
> .domainIsUpdated = libxlDomainIsUpdated, /* 0.9.0 */
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list