[libvirt] [PATCH v4 0/7] Add new APIs to edit xml configuration of managed save state of a domain

managedsave command offloads the user from managing the save state file. It does not need the user to specify saved state file location, all it takes is domain name to identify. This makes it much more comfortable to use in emergency where immediate shutdowm is needed. But it doesn't provide a way to edit XML description of the save state file without user going through an extra effort to search manually where the file actually exists. The series aims to overcome the above constraints by adding new APIs and commands to seemlessly edit the managed save state XML description using just the domain name. The Patches mainly make use of the save-image-edit code flow only to simplify the above use case. This patch set provides capability to Dump and Edit the XML configuration associated with a saved state file of a domain which was created by the managedsave command. The new command carry the similar options as the save-image-<XXX> commands to change the running state as to paused state or running on start. This is equivalent to: virsh managedsave-dumpxml domain-name > state-file.xml vi state-file.xml (or make changes with your other text editor) virsh managedsave-define domain-name state-file-xml or you can simply use: virsh managedsave-edit domain-name It's always better when we get more. Changes since v3: - refracted version references from 3.6.0 to 3.7.0 - fixed typo in error message. Changes since v2: - refracted version references from 3.5.0 to 3.6.0 Changes since v1: - qemu implementation called directly rather than going through driver pointer in qemuDomainManagedSaveDefineXML. - check whether the managed save state file exists and report a error if it doesn't. Kothapally Madhu Pavan (7): lib: Add API to dump xml configuration of managed save state domain lib: Add API to edit domain's managed save state xml configuration qemu: Implement qemuDomainManagedSaveGetXMLDesc qemu: Implement qemuDomainManagedSaveDefineXML virsh: Implement managedsave-define command virsh: Implement managedsave-dumpxml command virsh: Implement managedsave-edit command include/libvirt/libvirt-domain.h | 6 ++ src/driver-hypervisor.h | 11 +++ src/libvirt-domain.c | 107 ++++++++++++++++++++ src/libvirt_public.syms | 6 ++ src/qemu/qemu_driver.c | 87 ++++++++++++++++ src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 31 +++++- src/remote_protocol-structs | 14 +++ tools/virsh-domain.c | 207 +++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 41 ++++++++ 10 files changed, 511 insertions(+), 1 deletion(-) -- 1.8.3.1

Similar to domainSaveImageGetXMLDesc this commit adds domainManagedSaveGetXMLDesc API which allows to get the xml of managed save state domain. Signed-off-by: Kothapally Madhu Pavan <kmp@linux.vnet.ibm.com> --- include/libvirt/libvirt-domain.h | 2 ++ src/driver-hypervisor.h | 5 ++++ src/libvirt-domain.c | 49 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 19 ++++++++++++++-- src/remote_protocol-structs | 8 +++++++ 7 files changed, 87 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index a3bb9cb..8ede912 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1209,6 +1209,8 @@ int virDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags); int virDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags); +char * virDomainManagedSaveGetXMLDesc(virDomainPtr domain, + unsigned int flags); /* * Domain core dump diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 3053d7a..598fc06 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -745,6 +745,10 @@ typedef int (*virDrvDomainManagedSaveRemove)(virDomainPtr domain, unsigned int flags); +typedef char * +(*virDrvDomainManagedSaveGetXMLDesc)(virDomainPtr domain, + unsigned int flags); + typedef virDomainSnapshotPtr (*virDrvDomainSnapshotCreateXML)(virDomainPtr domain, const char *xmlDesc, @@ -1422,6 +1426,7 @@ struct _virHypervisorDriver { virDrvDomainManagedSave domainManagedSave; virDrvDomainHasManagedSaveImage domainHasManagedSaveImage; virDrvDomainManagedSaveRemove domainManagedSaveRemove; + virDrvDomainManagedSaveGetXMLDesc domainManagedSaveGetXMLDesc; virDrvDomainSnapshotCreateXML domainSnapshotCreateXML; virDrvDomainSnapshotGetXMLDesc domainSnapshotGetXMLDesc; virDrvDomainSnapshotNum domainSnapshotNum; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 87fca29..cb260e2 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -9298,6 +9298,55 @@ virDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags) } +/** + * virDomainManagedSaveGetXMLDesc: + * @domain: a domain object + * @flags: bitwise-OR of subset of virDomainXMLFlags + * + * This method will extract the XML description of the managed save + * state file of a domain. + * + * No security-sensitive data will be included unless @flags contains + * VIR_DOMAIN_XML_SECURE; this flag is rejected on read-only + * connections. For this API, @flags should not contain either + * VIR_DOMAIN_XML_INACTIVE or VIR_DOMAIN_XML_UPDATE_CPU. + * + * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of + * error. The caller must free() the returned value. + */ +char * +virDomainManagedSaveGetXMLDesc(virDomainPtr domain, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "flags=%x", flags); + + virResetLastError(); + + virCheckDomainReturn(domain, NULL); + conn = domain->conn; + + if ((conn->flags & VIR_CONNECT_RO) && (flags & VIR_DOMAIN_XML_SECURE)) { + virReportError(VIR_ERR_OPERATION_DENIED, "%s", + _("virDomainManagedSaveGetXMLDesc with secure flag")); + goto error; + } + + if (conn->driver->domainManagedSaveGetXMLDesc) { + char *ret; + ret = conn->driver->domainManagedSaveGetXMLDesc(domain, flags); + if (!ret) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return NULL; +} + /** * virDomainOpenConsole: diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index fac77fb..b38d0bb 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -768,4 +768,9 @@ LIBVIRT_3.4.0 { virStreamSparseSendAll; } LIBVIRT_3.1.0; +LIBVIRT_3.7.0 { + global: + virDomainManagedSaveGetXMLDesc; +} LIBVIRT_3.4.0; + # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index a57d25f..7a74b87 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8410,6 +8410,7 @@ static virHypervisorDriver hypervisor_driver = { .domainManagedSave = remoteDomainManagedSave, /* 0.8.0 */ .domainHasManagedSaveImage = remoteDomainHasManagedSaveImage, /* 0.8.0 */ .domainManagedSaveRemove = remoteDomainManagedSaveRemove, /* 0.8.0 */ + .domainManagedSaveGetXMLDesc = remoteDomainManagedSaveGetXMLDesc, /* 3.7.0 */ .domainSnapshotCreateXML = remoteDomainSnapshotCreateXML, /* 0.8.0 */ .domainSnapshotGetXMLDesc = remoteDomainSnapshotGetXMLDesc, /* 0.8.0 */ .domainSnapshotNum = remoteDomainSnapshotNum, /* 0.8.0 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 0943208..67d518c 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2547,6 +2547,15 @@ struct remote_domain_managed_save_remove_args { unsigned int flags; }; +struct remote_domain_managed_save_get_xml_desc_args { + remote_nonnull_domain dom; + unsigned int flags; +}; + +struct remote_domain_managed_save_get_xml_desc_ret { + remote_nonnull_string xml; +}; + struct remote_domain_snapshot_create_xml_args { remote_nonnull_domain dom; remote_nonnull_string xml_desc; @@ -6064,7 +6073,13 @@ enum remote_procedure { * @generate: both * @acl: domain:write */ - REMOTE_PROC_DOMAIN_SET_BLOCK_THRESHOLD = 386 - + REMOTE_PROC_DOMAIN_SET_BLOCK_THRESHOLD = 386, + /** + * @generate: both + * @priority: high + * @acl: domain:read + * @acl: domain:read_secure:VIR_DOMAIN_XML_SECURE + */ + REMOTE_PROC_DOMAIN_MANAGED_SAVE_GET_XML_DESC = 387 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index a46fe37..27f95ac 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1963,6 +1963,13 @@ struct remote_domain_managed_save_remove_args { remote_nonnull_domain dom; u_int flags; }; +struct remote_domain_managed_save_get_xml_desc_args { + remote_nonnull_domain dom; + u_int flags; +}; +struct remote_domain_managed_save_get_xml_desc_ret { + remote_nonnull_string xml; +}; struct remote_domain_snapshot_create_xml_args { remote_nonnull_domain dom; remote_nonnull_string xml_desc; @@ -3233,4 +3240,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SET_VCPU = 384, REMOTE_PROC_DOMAIN_EVENT_BLOCK_THRESHOLD = 385, REMOTE_PROC_DOMAIN_SET_BLOCK_THRESHOLD = 386, + REMOTE_PROC_DOMAIN_MANAGED_SAVE_GET_XML_DESC = 387 }; -- 1.8.3.1

Similar to domainSaveImageDefineXML this commit adds domainManagedSaveDefineXML API which allows to edit domain's managed save state xml configuration. Signed-off-by: Kothapally Madhu Pavan <kmp@linux.vnet.ibm.com> --- include/libvirt/libvirt-domain.h | 4 +++ src/driver-hypervisor.h | 6 +++++ src/libvirt-domain.c | 58 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 16 ++++++++++- src/remote_protocol-structs | 8 +++++- 7 files changed, 92 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 8ede912..ed2c2b6 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1211,6 +1211,10 @@ int virDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags); char * virDomainManagedSaveGetXMLDesc(virDomainPtr domain, unsigned int flags); +int virDomainManagedSaveDefineXML(virDomainPtr domain, + const char *dxml, + unsigned int flags); + /* * Domain core dump diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 598fc06..0a4181e 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -749,6 +749,11 @@ typedef char * (*virDrvDomainManagedSaveGetXMLDesc)(virDomainPtr domain, unsigned int flags); +typedef int +(*virDrvDomainManagedSaveDefineXML)(virDomainPtr domain, + const char *dxml, + unsigned int flags); + typedef virDomainSnapshotPtr (*virDrvDomainSnapshotCreateXML)(virDomainPtr domain, const char *xmlDesc, @@ -1427,6 +1432,7 @@ struct _virHypervisorDriver { virDrvDomainHasManagedSaveImage domainHasManagedSaveImage; virDrvDomainManagedSaveRemove domainManagedSaveRemove; virDrvDomainManagedSaveGetXMLDesc domainManagedSaveGetXMLDesc; + virDrvDomainManagedSaveDefineXML domainManagedSaveDefineXML; virDrvDomainSnapshotCreateXML domainSnapshotCreateXML; virDrvDomainSnapshotGetXMLDesc domainSnapshotGetXMLDesc; virDrvDomainSnapshotNum domainSnapshotNum; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index cb260e2..918e81a 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -9349,6 +9349,64 @@ virDomainManagedSaveGetXMLDesc(virDomainPtr domain, unsigned int flags) /** + * virDomainManagedSaveDefineXML: + * @domain: a domain object + * @dxml: XML config for adjusting guest xml used on restore + * @flags: bitwise-OR of virDomainSaveRestoreFlags + * + * This updates the definition of a domain stored in a saved state + * file. @domain is used to extract the saved state file location. + * + * @dxml can be used to alter host-specific portions of the domain XML + * that will be used on the next start of the domain. For example, it is + * possible to alter the backing filename that is associated with a + * disk device, to match renaming done as part of backing up the disk + * device while the domain is stopped. + * + * Normally, the saved state file will remember whether the domain was + * running or paused, and restore defaults to the same state. + * Specifying VIR_DOMAIN_SAVE_RUNNING or VIR_DOMAIN_SAVE_PAUSED in + * @flags will override the default saved into the file; omitting both + * leaves the file's default unchanged. These two flags are mutually + * exclusive. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virDomainManagedSaveDefineXML(virDomainPtr domain, const char *dxml, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "flags=%x", flags); + + virResetLastError(); + + VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SAVE_RUNNING, + VIR_DOMAIN_SAVE_PAUSED, + error); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + if (conn->driver->domainManagedSaveDefineXML) { + int ret; + ret = conn->driver->domainManagedSaveDefineXML(domain, dxml, flags); + + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} + + +/** * virDomainOpenConsole: * @dom: a domain object * @dev_name: the console, serial or parallel port device alias, or NULL diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index b38d0bb..fc4efd9 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -771,6 +771,7 @@ LIBVIRT_3.4.0 { LIBVIRT_3.7.0 { global: virDomainManagedSaveGetXMLDesc; + virDomainManagedSaveDefineXML; } LIBVIRT_3.4.0; # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 7a74b87..482d4ce 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8411,6 +8411,7 @@ static virHypervisorDriver hypervisor_driver = { .domainHasManagedSaveImage = remoteDomainHasManagedSaveImage, /* 0.8.0 */ .domainManagedSaveRemove = remoteDomainManagedSaveRemove, /* 0.8.0 */ .domainManagedSaveGetXMLDesc = remoteDomainManagedSaveGetXMLDesc, /* 3.7.0 */ + .domainManagedSaveDefineXML = remoteDomainManagedSaveDefineXML, /* 3.7.0 */ .domainSnapshotCreateXML = remoteDomainSnapshotCreateXML, /* 0.8.0 */ .domainSnapshotGetXMLDesc = remoteDomainSnapshotGetXMLDesc, /* 0.8.0 */ .domainSnapshotNum = remoteDomainSnapshotNum, /* 0.8.0 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 67d518c..ddc6bda 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2556,6 +2556,12 @@ struct remote_domain_managed_save_get_xml_desc_ret { remote_nonnull_string xml; }; +struct remote_domain_managed_save_define_xml_args { + remote_nonnull_domain dom; + remote_string dxml; + unsigned int flags; +}; + struct remote_domain_snapshot_create_xml_args { remote_nonnull_domain dom; remote_nonnull_string xml_desc; @@ -6081,5 +6087,13 @@ enum remote_procedure { * @acl: domain:read * @acl: domain:read_secure:VIR_DOMAIN_XML_SECURE */ - REMOTE_PROC_DOMAIN_MANAGED_SAVE_GET_XML_DESC = 387 + REMOTE_PROC_DOMAIN_MANAGED_SAVE_GET_XML_DESC = 387, + + /** + * @generate: both + * @priority: high + * @acl: domain:write + * @acl: domain:hibernate + */ + REMOTE_PROC_DOMAIN_MANAGED_SAVE_DEFINE_XML = 388 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 27f95ac..8075335 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1970,6 +1970,11 @@ struct remote_domain_managed_save_get_xml_desc_args { struct remote_domain_managed_save_get_xml_desc_ret { remote_nonnull_string xml; }; +struct remote_domain_managed_save_define_xml_args { + remote_nonnull_domain dom; + remote_nonnull_string dxml; + u_int flags; +}; struct remote_domain_snapshot_create_xml_args { remote_nonnull_domain dom; remote_nonnull_string xml_desc; @@ -3240,5 +3245,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SET_VCPU = 384, REMOTE_PROC_DOMAIN_EVENT_BLOCK_THRESHOLD = 385, REMOTE_PROC_DOMAIN_SET_BLOCK_THRESHOLD = 386, - REMOTE_PROC_DOMAIN_MANAGED_SAVE_GET_XML_DESC = 387 + REMOTE_PROC_DOMAIN_MANAGED_SAVE_GET_XML_DESC = 387, + REMOTE_PROC_DOMAIN_MANAGED_SAVE_DEFINE_XML = 388 }; -- 1.8.3.1

On Tue, Aug 08, 2017 at 13:32:50 +0530, Kothapally Madhu Pavan wrote:
Similar to domainSaveImageDefineXML this commit adds domainManagedSaveDefineXML API which allows to edit domain's managed save state xml configuration.
Signed-off-by: Kothapally Madhu Pavan <kmp@linux.vnet.ibm.com> --- include/libvirt/libvirt-domain.h | 4 +++ src/driver-hypervisor.h | 6 +++++ src/libvirt-domain.c | 58 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 16 ++++++++++- src/remote_protocol-structs | 8 +++++- 7 files changed, 92 insertions(+), 2 deletions(-)
[...]
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 67d518c..ddc6bda 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2556,6 +2556,12 @@ struct remote_domain_managed_save_get_xml_desc_ret { remote_nonnull_string xml; };
+struct remote_domain_managed_save_define_xml_args { + remote_nonnull_domain dom; + remote_string dxml;
This ...
+ unsigned int flags; +}; + struct remote_domain_snapshot_create_xml_args { remote_nonnull_domain dom; remote_nonnull_string xml_desc;
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 27f95ac..8075335 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1970,6 +1970,11 @@ struct remote_domain_managed_save_get_xml_desc_args { struct remote_domain_managed_save_get_xml_desc_ret { remote_nonnull_string xml; }; +struct remote_domain_managed_save_define_xml_args { + remote_nonnull_domain dom; + remote_nonnull_string dxml;
... and this does not match, the build has failed.
+ u_int flags; +};
More comments perhaps later, I've fixed this in my tree.

This commit adds qemu driver implementation to get xml description for managed save state domain. Signed-off-by: Kothapally Madhu Pavan <kmp@linux.vnet.ibm.com> --- src/qemu/qemu_driver.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b3f65f4..ec73dc1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6797,6 +6797,51 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, return ret; } +static char * +qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm; + char *path = NULL; + char *ret = NULL; + virDomainDefPtr def = NULL; + int fd = -1; + virQEMUSaveDataPtr data = NULL; + + /* We only take subset of virDomainDefFormat flags. */ + virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL); + + if (!(vm = qemuDomObjFromDomain(dom))) + return ret; + + path = qemuDomainManagedSavePath(driver, vm); + + if (!path) + goto cleanup; + + if (!virFileExists(path)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s",_("domain does not have managed save image")); + goto cleanup; + } + + fd = qemuDomainSaveImageOpen(driver, path, &def, &data, + false, NULL, false, false); + if (fd < 0) + goto cleanup; + if (virDomainManagedSaveGetXMLDescEnsureACL(dom->conn, def, flags) < 0) + goto cleanup; + ret = qemuDomainDefFormatXML(driver, def, flags); + + cleanup: + virQEMUSaveDataFree(data); + virDomainDefFree(def); + VIR_FORCE_CLOSE(fd); + virDomainObjEndAPI(&vm); + VIR_FREE(path); + return ret; +} + /* Return 0 on success, 1 if incomplete saved image was silently unlinked, * and -1 on failure with error raised. */ static int @@ -20839,6 +20884,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainManagedSave = qemuDomainManagedSave, /* 0.8.0 */ .domainHasManagedSaveImage = qemuDomainHasManagedSaveImage, /* 0.8.0 */ .domainManagedSaveRemove = qemuDomainManagedSaveRemove, /* 0.8.0 */ + .domainManagedSaveGetXMLDesc = qemuDomainManagedSaveGetXMLDesc, /* 3.7.0 */ .domainSnapshotCreateXML = qemuDomainSnapshotCreateXML, /* 0.8.0 */ .domainSnapshotGetXMLDesc = qemuDomainSnapshotGetXMLDesc, /* 0.8.0 */ .domainSnapshotNum = qemuDomainSnapshotNum, /* 0.8.0 */ -- 1.8.3.1

On Tue, Aug 08, 2017 at 13:32:51 +0530, Kothapally Madhu Pavan wrote:
This commit adds qemu driver implementation to get xml description for managed save state domain.
Signed-off-by: Kothapally Madhu Pavan <kmp@linux.vnet.ibm.com> --- src/qemu/qemu_driver.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b3f65f4..ec73dc1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6797,6 +6797,51 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, return ret; }
+static char * +qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm; + char *path = NULL; + char *ret = NULL; + virDomainDefPtr def = NULL; + int fd = -1; + virQEMUSaveDataPtr data = NULL; + + /* We only take subset of virDomainDefFormat flags. */ + virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL); + + if (!(vm = qemuDomObjFromDomain(dom))) + return ret; + + path = qemuDomainManagedSavePath(driver, vm); + + if (!path) + goto cleanup; + + if (!virFileExists(path)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s",_("domain does not have managed save image"));
You did not run syntax-check on this patch: Invalid character after comma: src/qemu/qemu_driver.c:6824: "%s",_("domain does not have managed save image"));

On Tue, Aug 08, 2017 at 13:32:51 +0530, Kothapally Madhu Pavan wrote:
This commit adds qemu driver implementation to get xml description for managed save state domain.
Signed-off-by: Kothapally Madhu Pavan <kmp@linux.vnet.ibm.com> --- src/qemu/qemu_driver.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b3f65f4..ec73dc1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6797,6 +6797,51 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, return ret; }
+static char * +qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm; + char *path = NULL; + char *ret = NULL; + virDomainDefPtr def = NULL; + int fd = -1; + virQEMUSaveDataPtr data = NULL; + + /* We only take subset of virDomainDefFormat flags. */ + virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL); + + if (!(vm = qemuDomObjFromDomain(dom))) + return ret; + + path = qemuDomainManagedSavePath(driver, vm); + + if (!path) + goto cleanup; + + if (!virFileExists(path)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s",_("domain does not have managed save image")); + goto cleanup; + } + + fd = qemuDomainSaveImageOpen(driver, path, &def, &data, + false, NULL, false, false); + if (fd < 0) + goto cleanup; + if (virDomainManagedSaveGetXMLDescEnsureACL(dom->conn, def, flags) < 0) + goto cleanup;
Since you have the 'vm' object at the beginning, I think the ACL check should be done right away with vm->def in this case. The ACL check should only need the name and UUID from the definition and thus can be run earlier. This will mitigate a possible side channel, where we'd return 'domain does not have managed save image' instead of the "access denied" message. I'll do this adjustment locally along with others pointed out. I might finish this until the freeze tomorrow.

This commit adds qemu driver implementation to edit xml configuration of managed save state file of a domain. Signed-off-by: Kothapally Madhu Pavan <kmp@linux.vnet.ibm.com> --- src/qemu/qemu_driver.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ec73dc1..b6db435 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6842,6 +6842,46 @@ qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, unsigned int flags) return ret; } +static int +qemuDomainManagedSaveDefineXML(virDomainPtr dom, const char *dxml, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virConnectPtr conn = dom->conn; + virDomainObjPtr vm; + char *path = NULL; + int ret; + + if (!(vm = qemuDomObjFromDomain(dom))) + return -1; + + path = qemuDomainManagedSavePath(driver, vm); + virDomainObjEndAPI(&vm); + + if (!path) + goto error; + + if (!virFileExists(path)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s",_("domain does not have managed save image")); + goto error; + } + + ret = qemuDomainSaveImageDefineXML(conn, path, dxml, flags); + + VIR_FREE(path); + + if (ret < 0) + goto error; + + return ret; + + error: + VIR_FREE(path); + virDispatchError(conn); + return -1; +} + /* Return 0 on success, 1 if incomplete saved image was silently unlinked, * and -1 on failure with error raised. */ static int @@ -20885,6 +20925,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainHasManagedSaveImage = qemuDomainHasManagedSaveImage, /* 0.8.0 */ .domainManagedSaveRemove = qemuDomainManagedSaveRemove, /* 0.8.0 */ .domainManagedSaveGetXMLDesc = qemuDomainManagedSaveGetXMLDesc, /* 3.7.0 */ + .domainManagedSaveDefineXML = qemuDomainManagedSaveDefineXML, /* 3.7.0 */ .domainSnapshotCreateXML = qemuDomainSnapshotCreateXML, /* 0.8.0 */ .domainSnapshotGetXMLDesc = qemuDomainSnapshotGetXMLDesc, /* 0.8.0 */ .domainSnapshotNum = qemuDomainSnapshotNum, /* 0.8.0 */ -- 1.8.3.1

On Tue, Aug 08, 2017 at 13:32:52 +0530, Kothapally Madhu Pavan wrote:
This commit adds qemu driver implementation to edit xml configuration of managed save state file of a domain.
Signed-off-by: Kothapally Madhu Pavan <kmp@linux.vnet.ibm.com> --- src/qemu/qemu_driver.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ec73dc1..b6db435 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6842,6 +6842,46 @@ qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, unsigned int flags) return ret; }
+static int +qemuDomainManagedSaveDefineXML(virDomainPtr dom, const char *dxml, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virConnectPtr conn = dom->conn; + virDomainObjPtr vm; + char *path = NULL; + int ret; + + if (!(vm = qemuDomObjFromDomain(dom))) + return -1; + + path = qemuDomainManagedSavePath(driver, vm); + virDomainObjEndAPI(&vm); + + if (!path) + goto error; + + if (!virFileExists(path)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s",_("domain does not have managed save image")); + goto error; + } + + ret = qemuDomainSaveImageDefineXML(conn, path, dxml, flags); + + VIR_FREE(path); + + if (ret < 0) + goto error; + + return ret; + + error: + VIR_FREE(path); + virDispatchError(conn);
Error dispatching is supposed to be done from the main API call where you've added, so it does not need to be duplicated. Also it's possible to just return ret, since it's set correctly in qemuDomainSaveImageDefineXML.

Add a simple virsh command handler which makes use of the new API. Signed-off-by: Kothapally Madhu Pavan <kmp@linux.vnet.ibm.com> --- tools/virsh-domain.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 14 ++++++++++ 2 files changed, 93 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 512804c..8baea2a 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4705,6 +4705,79 @@ cmdManagedSaveRemove(vshControl *ctl, const vshCmd *cmd) } /* + * "managedsave-define" command + */ +static const vshCmdInfo info_managed_save_define[] = { + {.name = "help", + .data = N_("redefine the XML for a domain's managed save state file") + }, + {.name = "desc", + .data = N_("Replace the domain XML associated with a managed save state file") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_managed_save_define[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL, + {.name = "xml", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("filename containing updated XML for the target") + }, + {.name = "running", + .type = VSH_OT_BOOL, + .help = N_("set domain to be running on start") + }, + {.name = "paused", + .type = VSH_OT_BOOL, + .help = N_("set domain to be paused on start") + }, + {.name = NULL} +}; + +static bool +cmdManagedSaveDefine(vshControl *ctl, const vshCmd *cmd) +{ + bool ret = false; + virDomainPtr dom = NULL; + const char *xmlfile = NULL; + char *xml = NULL; + unsigned int flags = 0; + + if (vshCommandOptBool(cmd, "running")) + flags |= VIR_DOMAIN_SAVE_RUNNING; + if (vshCommandOptBool(cmd, "paused")) + flags |= VIR_DOMAIN_SAVE_PAUSED; + + VSH_EXCLUSIVE_OPTIONS("running", "paused"); + + if (vshCommandOptStringReq(ctl, cmd, "xml", &xmlfile) < 0) + return false; + + if (virFileReadAll(xmlfile, VSH_MAX_XML_FILE, &xml) < 0) + return false; + + dom = virshCommandOptDomain(ctl, cmd, NULL); + if (dom == NULL) + goto cleanup; + + if (virDomainManagedSaveDefineXML(dom, xml, flags) < 0) { + vshError(ctl, _("Failed to update %s XML configuration"), + virDomainGetName(dom)); + goto cleanup; + } + + vshPrintExtra(ctl, _("Managed save state file of domain %s updated.\n"), + virDomainGetName(dom)); + ret = true; + + cleanup: + virshDomainFree(dom); + VIR_FREE(xml); + return ret; +} + +/* * "schedinfo" command */ static const vshCmdInfo info_schedinfo[] = { @@ -13845,6 +13918,12 @@ const vshCmdDef domManagementCmds[] = { .info = info_managedsaveremove, .flags = 0 }, + {.name = "managedsave-define", + .handler = cmdManagedSaveDefine, + .opts = opts_managed_save_define, + .info = info_managed_save_define, + .flags = 0 + }, {.name = "memtune", .handler = cmdMemtune, .opts = opts_memtune, diff --git a/tools/virsh.pod b/tools/virsh.pod index 43d6f0c..11e2321 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1630,6 +1630,20 @@ has any managed save image. Remove the B<managedsave> state file for a domain, if it exists. This ensures the domain will do a full boot the next time it is started. +=item B<managedsave-define> I<domain> I<xml> [{I<--running> | I<--paused>}] + +Update the domain XML that will be used when I<domain> is later +started. The I<xml> argument must be a file name containing +the alternative XML, with changes only in the host-specific portions of +the domain XML. For example, it can be used to account for file naming +differences resulting from creating disk snapshots of underlying storage +after the guest was saved. + +The managed save image records whether the domain should be started to a +running or paused state. Normally, this command does not alter the +recorded state; passing either the I<--running> or I<--paused> flag +will allow overriding which state the B<start> should use. + =item B<maxvcpus> [I<type>] Provide the maximum number of virtual CPUs supported for a guest VM on -- 1.8.3.1

Add a simple virsh command handler which makes use of the new API. Signed-off-by: Kothapally Madhu Pavan <kmp@linux.vnet.ibm.com> --- tools/virsh-domain.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 6 ++++++ 2 files changed, 62 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 8baea2a..ce15090 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4705,6 +4705,56 @@ cmdManagedSaveRemove(vshControl *ctl, const vshCmd *cmd) } /* + * "managedsave-dumpxml" command + */ +static const vshCmdInfo info_managed_save_dumpxml[] = { + {.name = "help", + .data = N_("Domain information of managed save state file in XML") + }, + {.name = "desc", + .data = N_("Dump XML of domain information for a managed save state file to stdout.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_managed_save_dumpxml[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL, + {.name = "security-info", + .type = VSH_OT_BOOL, + .help = N_("include security sensitive information in XML dump") + }, + {.name = NULL} +}; + +static bool +cmdManagedSaveDumpxml(vshControl *ctl, const vshCmd *cmd) +{ + bool ret = false; + virDomainPtr dom = NULL; + unsigned int flags = 0; + char *xml = NULL; + + if (vshCommandOptBool(cmd, "security-info")) + flags |= VIR_DOMAIN_XML_SECURE; + + dom = virshCommandOptDomain(ctl, cmd, NULL); + if (dom == NULL) + goto cleanup; + + xml = virDomainManagedSaveGetXMLDesc(dom, flags); + if (!xml) + goto cleanup; + + vshPrint(ctl, "%s", xml); + ret = true; + + cleanup: + virshDomainFree(dom); + VIR_FREE(xml); + return ret; +} + +/* * "managedsave-define" command */ static const vshCmdInfo info_managed_save_define[] = { @@ -13918,6 +13968,12 @@ const vshCmdDef domManagementCmds[] = { .info = info_managedsaveremove, .flags = 0 }, + {.name = "managedsave-dumpxml", + .handler = cmdManagedSaveDumpxml, + .opts = opts_managed_save_dumpxml, + .info = info_managed_save_dumpxml, + .flags = 0 + }, {.name = "managedsave-define", .handler = cmdManagedSaveDefine, .opts = opts_managed_save_define, diff --git a/tools/virsh.pod b/tools/virsh.pod index 11e2321..1bda44f 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1644,6 +1644,12 @@ running or paused state. Normally, this command does not alter the recorded state; passing either the I<--running> or I<--paused> flag will allow overriding which state the B<start> should use. +=item B<managedsave-dumpxml> I<domain> [I<--security-info>] + +Extract the domain XML that was in effect at the time the saved state +file I<file> was created with the B<managedsave> command. Using +I<--security-info> will also include security sensitive information. + =item B<maxvcpus> [I<type>] Provide the maximum number of virtual CPUs supported for a guest VM on -- 1.8.3.1

Add a simple virsh command handler which makes use of the new API. Signed-off-by: Kothapally Madhu Pavan <kmp@linux.vnet.ibm.com> --- tools/virsh-domain.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 21 +++++++++++++++ 2 files changed, 93 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index ce15090..6152661 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4705,6 +4705,72 @@ cmdManagedSaveRemove(vshControl *ctl, const vshCmd *cmd) } /* + * "managedsave-edit" command + */ +static const vshCmdInfo info_managed_save_edit[] = { + {.name = "help", + .data = N_("edit XML for a domain's managed save state file") + }, + {.name = "desc", + .data = N_("Edit the domain XML associated with the managed save state file") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_managed_save_edit[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL, + {.name = "running", + .type = VSH_OT_BOOL, + .help = N_("set domain to be running on start") + }, + {.name = "paused", + .type = VSH_OT_BOOL, + .help = N_("set domain to be paused on start") + }, + {.name = NULL} +}; + +static bool +cmdManagedSaveEdit(vshControl *ctl, const vshCmd *cmd) +{ + bool ret = false; + virDomainPtr dom = NULL; + unsigned int getxml_flags = VIR_DOMAIN_XML_SECURE; + unsigned int define_flags = 0; + + if (vshCommandOptBool(cmd, "running")) + define_flags |= VIR_DOMAIN_SAVE_RUNNING; + if (vshCommandOptBool(cmd, "paused")) + define_flags |= VIR_DOMAIN_SAVE_PAUSED; + + VSH_EXCLUSIVE_OPTIONS("running", "paused"); + + dom = virshCommandOptDomain(ctl, cmd, NULL); + if (dom == NULL) + goto cleanup; + +#define EDIT_GET_XML virDomainManagedSaveGetXMLDesc(dom, getxml_flags) +#define EDIT_NOT_CHANGED \ + do { \ + vshPrintExtra(ctl, _("Managed save image of domain %s XML configuration " \ + "not changed.\n"), virDomainGetName(dom)); \ + ret = true; \ + goto edit_cleanup; \ + } while (0) +#define EDIT_DEFINE \ + (virDomainManagedSaveDefineXML(dom, doc_edited, define_flags) == 0) +#include "virsh-edit.c" + + vshPrintExtra(ctl, _("Managed save image of Domain %s XML configuration edited.\n"), + virDomainGetName(dom)); + ret = true; + + cleanup: + virshDomainFree(dom); + return ret; +} + +/* * "managedsave-dumpxml" command */ static const vshCmdInfo info_managed_save_dumpxml[] = { @@ -13968,6 +14034,12 @@ const vshCmdDef domManagementCmds[] = { .info = info_managedsaveremove, .flags = 0 }, + {.name = "managedsave-edit", + .handler = cmdManagedSaveEdit, + .opts = opts_managed_save_edit, + .info = info_managed_save_edit, + .flags = 0 + }, {.name = "managedsave-dumpxml", .handler = cmdManagedSaveDumpxml, .opts = opts_managed_save_dumpxml, diff --git a/tools/virsh.pod b/tools/virsh.pod index 1bda44f..dbad9a0 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1650,6 +1650,27 @@ Extract the domain XML that was in effect at the time the saved state file I<file> was created with the B<managedsave> command. Using I<--security-info> will also include security sensitive information. +=item B<managedsave-edit> I<domain> [{I<--running> | I<--paused>}] + +Edit the XML configuration associated with a saved state file of a +I<domain> was created by the B<managedsave> command. + +The managed save image records whether the domain should be started to a +running or paused state. Normally, this command does not alter the +recorded state; passing either the I<--running> or I<--paused> flag +will allow overriding which state the B<restore> should use. + +This is equivalent to: + + virsh managedsave-dumpxml domain-name > state-file.xml + vi state-file.xml (or make changes with your other text editor) + virsh managedsave-define domain-name state-file-xml + +except that it does some error checking. + +The editor used can be supplied by the C<$VISUAL> or C<$EDITOR> environment +variables, and defaults to C<vi>. + =item B<maxvcpus> [I<type>] Provide the maximum number of virtual CPUs supported for a guest VM on -- 1.8.3.1
participants (2)
-
Kothapally Madhu Pavan
-
Peter Krempa