Re: [PATCH] Remove VIR_FREE in favor of g_autofree in some functions in libvrit-domain.c
Note that I had to drop the 'noreply.github.com' email address. Please
don't use that as the author credential as it makes hard to reply
properly to threads.
On Mon, Mar 11, 2024 at 02:15:32 +0200, Mostafa wrote:
> From: مصطفي محمود كمال الدين <48567303+most...@users.noreply.github.com>
Per contributor guidelines:
Contributors to libvirt projects must assert that they are in
compliance with the Developer Certificate of Origin 1.1. This is
achieved by adding a "Signed-off-by" line containing the contributor's
name and e-mail to every commit message. The presence of this line
attests that the contributor has read the above lined DCO and agrees
with its statements.
https://www.libvirt.org/hacking.html#developer-certificate-of-origin
Please make sure to follow all points of that document, especially all
paragraphs about:
https://www.libvirt.org/hacking.html#preparing-patches
in future postings.
> ---
> src/libvirt-domain.c | 32
> 1 file changed, 8 insertions(+), 24 deletions(-)
>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 83abad251e..9b68a7ac95 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -884,7 +884,7 @@ virDomainSave(virDomainPtr domain, const char *to)
>
> if (conn->driver->domainSave) {
> int ret;
> -char *absolute_to;
> +g_autofree char *absolute_to;
>
> /* We must absolutize the file path as the save is done out of
> process */
> if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
After this patch the code fails to compile:
In function ‘g_autoptr_cleanup_generic_gfree’,
inlined from ‘virDomainSave’ at ../../../libvirt/src/libvirt-domain.c:887:2
:
/usr/include/glib-2.0/glib/glib-autocleanups.h:30:3: error: ‘absolute_to’ may
be used uninitialized [-Werror=maybe-uninitialized]
30 | g_free (*pp);
| ^~~~
../../../libvirt/src/libvirt-domain.c: In function ‘virDomainSave’:
../../../libvirt/src/libvirt-domain.c:887:26: note: ‘absolute_to’ was declared
here
887 | g_autofree char *absolute_to;
| ^~~
In function ‘g_autoptr_cleanup_generic_gfree’,
inlined from ‘virDomainSaveFlags’ at
../../../libvirt/src/libvirt-domain.c:975:26:
/usr/include/glib-2.0/glib/glib-autocleanups.h:30:3: error: ‘absolute_to’ may
be used uninitialized [-Werror=maybe-uninitialized]
30 | g_free (*pp);
| ^~~~
When compiling the code by default from a git checkout we enable all
errors thus you should be getting this error or the syntax-check error
for the same thing if you have an older compiler. Make sure to both
compile and run tests as the contributor guidelines state.
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] Remove VIR_FREE in favor of g_autofree in some functions in libvrit-domain.c
On Mon, Mar 11, 2024 at 02:15:32AM +0200, Mostafa wrote:
> From: مصطفي محمود كمال الدين <48567303+most...@users.noreply.github.com>
>
> ---
> src/libvirt-domain.c | 32
> 1 file changed, 8 insertions(+), 24 deletions(-)
>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 83abad251e..9b68a7ac95 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -884,7 +884,7 @@ virDomainSave(virDomainPtr domain, const char *to)
>
> if (conn->driver->domainSave) {
> int ret;
> -char *absolute_to;
> +g_autofree char *absolute_to;
All variables declared with 'g_autofree' *must* be initialized
at time of declaration, so you need to add ' = NULL' here and
to all the other similar changes.
>
> /* We must absolutize the file path as the save is done out of
> process */
> if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
> @@ -895,8 +895,6 @@ virDomainSave(virDomainPtr domain, const char *to)
>
> ret = conn->driver->domainSave(domain, absolute_to);
>
> -VIR_FREE(absolute_to);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -974,7 +972,7 @@ virDomainSaveFlags(virDomainPtr domain, const char *to,
>
> if (conn->driver->domainSaveFlags) {
> int ret;
> -char *absolute_to;
> +g_autofree char *absolute_to;
>
> /* We must absolutize the file path as the save is done out of
> process */
> if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
> @@ -985,8 +983,6 @@ virDomainSaveFlags(virDomainPtr domain, const char *to,
>
> ret = conn->driver->domainSaveFlags(domain, absolute_to, dxml,
> flags);
>
> -VIR_FREE(absolute_to);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -1076,7 +1072,7 @@ virDomainRestore(virConnectPtr conn, const char *from)
>
> if (conn->driver->domainRestore) {
> int ret;
> -char *absolute_from;
> +g_autofree char *absolute_from;
>
> /* We must absolutize the file path as the restore is done out of
> process */
> if (!(absolute_from = g_canonicalize_filename(from, NULL))) {
> @@ -1087,8 +1083,6 @@ virDomainRestore(virConnectPtr conn, const char *from)
>
> ret = conn->driver->domainRestore(conn, absolute_from);
>
> -VIR_FREE(absolute_from);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -1156,7 +1150,7 @@ virDomainRestoreFlags(virConnectPtr conn, const char
> *from, const char *dxml,
>
> if (conn->driver->domainRestoreFlags) {
> int ret;
> -char *absolute_from;
> +g_autofree char *absolute_from;
>
> /* We must absolutize the file path as the restore is done out of
> process */
> if (!(absolute_from = g_canonicalize_filename(from, NULL))) {
> @@ -1168,8 +1162,6 @@ virDomainRestoreFlags(virConnectPtr conn, const char
> *from, const char *dxml,
> ret = conn->driver->domainRestoreFlags(conn, absolute_from, dxml,
> flags);
>
> -VIR_FREE(absolute_from);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -1263,7 +1255,7 @@ virDomainSaveImageGetXMLDesc(virConnectPtr conn, const
> char *file,
>
> if (conn->driver->domainSaveImageGetXMLDesc) {
> char *ret;
> -char *absolute_file;
> +g_autofree char *absolute_file;
>
> /* We must absolutize the file path as the read is done out of
> process */
> if (!(absolute_file = g_canonicalize_filename(file, NULL))) {
> @@ -1275,8 +1267,6 @@ virDomainSaveImageGetXMLDesc(virConnectPtr conn, const
> char *file,
> ret = conn->driver->domainSaveImageGetXMLDesc(conn, absolute_file,
>flags);
>
> -VIR_FREE(absolute_file);
> -
> if (!ret)
> goto error;
> return ret;
> @@ -1338,7 +1328,7 @@ virDomainSaveImageDefineXML(virConnectPtr conn, const
> char *file,
>
> if (conn->driver->domainSaveImageDefineXML) {
> int ret;
> -char *absolute_file;
> +g_autofree char *absolute_file;
>
> /* We must absolutize the file path as the read is done out of
> process */
> if (!(absolute_file = g_canonicalize_filename(file, NULL))) {
> @@ -1350,8 +1340,6 @@ virDomainSaveImageDefineXML(virConnectPtr conn, const
> char *file,
> ret = conn->driver->domainSaveImageDefineXML(conn, absolute_file,
> dxml, flags);
>
> -VIR_FREE(absolute_file);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -1415,7 +1403,7 @@ virDomainCoreDump(virDomainPtr domain, const char *to,
> unsigned int flags)
>
> if (conn->driver->domainCoreDump) {
> int ret;
> -
[PATCH 00/11] ch_driver: Add basic SAVE and RESTORE VM operations support to CH driver
save, managedsave and restore operations for ch_driver are now supported for domains without any network, hostdev config defined. The input `path` to save and restore commands should be a directory path as cloud-hypervisor expects dir path where it saves VM state and other config files. Purna Pavan Chandra Aekkaladevi (11): ch_driver: Support Save, Restore VM actions from monitor ch_driver: Pass virCHDriverConfig to virCHMonitorNew ch_driver: Add domainSave, domainSaveFlags callbacks ch_driver: Add domainManagedSave callback ch_driver: Implement more save callbacks ch_driver: Refactor virCHProcessStart ch_driver: Implement domain restore callbacks ch_driver: cleanup any stale managed save dir before VM creation ch_driver: Add additional validation for save/restore docs: Update doc for virDomainSave and virDomainRestore NEWS: Mention save & restore support for ch driver NEWS.rst | 5 + src/ch/ch_conf.c | 6 + src/ch/ch_conf.h | 12 + src/ch/ch_driver.c | 515 ++- src/ch/ch_monitor.c | 97 +++- src/ch/ch_monitor.h | 6 +- src/ch/ch_process.c | 102 +++-- src/ch/ch_process.h | 4 + src/libvirt-domain.c | 10 +- 9 files changed, 730 insertions(+), 27 deletions(-) -- 2.34.1 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 02/11] ch_driver: Pass virCHDriverConfig to virCHMonitorNew
Pass virCHDriverConfig to VirCHMonitorNew instead of just stateDir so
that the cfg can be used for any additional purposes.
Signed-off-by: Purna Pavan Chandra Aekkaladevi
---
src/ch/ch_monitor.c | 3 ++-
src/ch/ch_monitor.h | 2 +-
src/ch/ch_process.c | 2 +-
3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index facbff002e..939fa13667 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -529,10 +529,11 @@ chMonitorCreateSocket(const char *socket_path)
}
virCHMonitor *
-virCHMonitorNew(virDomainObj *vm, const char *socketdir)
+virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg)
{
g_autoptr(virCHMonitor) mon = NULL;
g_autoptr(virCommand) cmd = NULL;
+const char *socketdir = cfg->stateDir;
int socket_fd = 0;
if (virCHMonitorInitialize() < 0)
diff --git a/src/ch/ch_monitor.h b/src/ch/ch_monitor.h
index 3e0befe5d0..ea6b2a771b 100644
--- a/src/ch/ch_monitor.h
+++ b/src/ch/ch_monitor.h
@@ -101,7 +101,7 @@ struct _virCHMonitor {
virCHMonitorThreadInfo *threads;
};
-virCHMonitor *virCHMonitorNew(virDomainObj *vm, const char *socketdir);
+virCHMonitor *virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg);
void virCHMonitorClose(virCHMonitor *mon);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCHMonitor, virCHMonitorClose);
diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c
index 7488b1d65d..4b360413fb 100644
--- a/src/ch/ch_process.c
+++ b/src/ch/ch_process.c
@@ -51,7 +51,7 @@ virCHProcessConnectMonitor(virCHDriver *driver,
virCHMonitor *monitor = NULL;
virCHDriverConfig *cfg = virCHDriverGetConfig(driver);
-monitor = virCHMonitorNew(vm, cfg->stateDir);
+monitor = virCHMonitorNew(vm, cfg);
virObjectUnref(cfg);
return monitor;
--
2.34.1
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 03/11] ch_driver: Add domainSave, domainSaveFlags callbacks
Implemented save callbacks. CH's vmm.snapshot API is called to save the
domain state. The path passed to these callbacks has to be of directory
as CH takes dir as input to snapshot and saves multiple files under it.
Signed-off-by: Purna Pavan Chandra Aekkaladevi
---
src/ch/ch_conf.h | 11
src/ch/ch_driver.c | 144 +
2 files changed, 155 insertions(+)
diff --git a/src/ch/ch_conf.h b/src/ch/ch_conf.h
index 579eca894e..4b4c3345b6 100644
--- a/src/ch/ch_conf.h
+++ b/src/ch/ch_conf.h
@@ -81,6 +81,17 @@ struct _virCHDriver
ebtablesContext *ebtables;
};
+#define CH_SAVE_MAGIC "libvirt-xml\n \0 \r"
+#define CH_SAVE_XML "libvirt-save.xml"
+
+typedef struct _CHSaveXMLHeader CHSaveXMLHeader;
+struct _CHSaveXMLHeader {
+char magic[sizeof(CH_SAVE_MAGIC)-1];
+uint32_t xmlLen;
+/* 20 bytes used, pad up to 64 bytes */
+uint32_t unused[11];
+};
+
virCaps *virCHDriverCapsInit(void);
virCaps *virCHDriverGetCapabilities(virCHDriver *driver,
bool refresh);
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 2601eea44b..24f697d2e1 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -19,6 +19,7 @@
*/
#include
+#include
#include "ch_capabilities.h"
#include "ch_conf.h"
@@ -34,6 +35,7 @@
#include "virerror.h"
#include "virlog.h"
#include "virobject.h"
+#include "virfile.h"
#include "virtypedparam.h"
#include "virutil.h"
#include "viruuid.h"
@@ -621,6 +623,146 @@ chDomainDestroy(virDomainPtr dom)
return chDomainDestroyFlags(dom, 0);
}
+/**
+ * chDoDomainSave:
+ * @driver: pointer to driver structure
+ * @vm: pointer to virtual machine structure. Must be locked before invocation.
+ * @to_dir: directory path (CH needs directory input) to save the domain
+ * @managed: whether the VM is managed or not
+ *
+ * Checks if the domain is running or paused, then suspends it and saves it
+ * using CH's vmm.snapshot API. CH creates multiple files for config, memory,
+ * device state into @to_dir.
+ *
+ * Returns 0 on success or -1 in case of error
+ */
+static int
+chDoDomainSave(virCHDriver *driver,
+ virDomainObj *vm,
+ const char *to_dir,
+ bool managed)
+{
+g_autoptr(virCHDriverConfig) cfg = virCHDriverGetConfig(driver);
+virCHDomainObjPrivate *priv = vm->privateData;
+CHSaveXMLHeader hdr;
+g_autofree char *to = NULL;
+g_autofree char *xml = NULL;
+uint32_t xml_len;
+VIR_AUTOCLOSE fd = -1;
+int ret = -1;
+
+virDomainState domainState = virDomainObjGetState(vm, NULL);
+if (domainState == VIR_DOMAIN_RUNNING) {
+if (virCHMonitorSuspendVM(priv->monitor) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to suspend domain before saving"));
+goto end;
+}
+virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_SAVE);
+} else if (domainState != VIR_DOMAIN_PAUSED) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("only can save running/paused domain"));
+goto end;
+}
+
+if (virDirCreate(to_dir, 0770, cfg->user, cfg->group,
+ VIR_DIR_CREATE_ALLOW_EXIST) < 0) {
+virReportSystemError(errno, _("Failed to create SAVE dir %1$s"),
to_dir);
+goto end;
+}
+
+to = g_strdup_printf("%s/%s", to_dir, CH_SAVE_XML);
+if ((fd = virFileOpenAs(to, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR,
+cfg->user, cfg->group, 0)) < 0) {
+virReportSystemError(-fd,
+ _("Failed to create/open domain save xml file
'%1$s'"),
+ to);
+goto end;
+}
+
+if ((xml = virDomainDefFormat(vm->def, driver->xmlopt, 0)) == NULL)
+goto end;
+xml_len = strlen(xml) + 1;
+
+memset(&hdr, 0, sizeof(hdr));
+memcpy(hdr.magic, CH_SAVE_MAGIC, sizeof(hdr.magic));
+hdr.xmlLen = xml_len;
+
+if (safewrite(fd, &hdr, sizeof(hdr)) != sizeof(hdr)) {
+virReportSystemError(errno, "%s", _("Failed to write file header"));
+goto end;
+}
+
+if (safewrite(fd, xml, xml_len) != xml_len) {
+virReportSystemError(errno, "%s", _("Failed to write xml definition"));
+goto end;
+}
+
+if (virCHMonitorSaveVM(priv->monitor, to_dir) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to save
domain"));
+goto end;
+}
+
+if (virCHProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_SAVED) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to shutoff after domain save"));
+goto end;
+}
+
+vm->hasManagedSave = managed;
+ret = 0;
+
+ end:
+return ret;
+}
+
+static int
+chDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml, unsigned
int flags)
+{
+virCHDriver *driver = dom->conn->privateData;
+virDomainObj *vm
[PATCH 04/11] ch_driver: Add domainManagedSave callback
Create libvirt managed saveDir and pass it to CH to save the VM
Signed-off-by: Purna Pavan Chandra Aekkaladevi
---
src/ch/ch_conf.c| 6 ++
src/ch/ch_conf.h| 1 +
src/ch/ch_driver.c | 51 +
src/ch/ch_monitor.c | 7 +++
4 files changed, 65 insertions(+)
diff --git a/src/ch/ch_conf.c b/src/ch/ch_conf.c
index a7b2285886..cab97639c4 100644
--- a/src/ch/ch_conf.c
+++ b/src/ch/ch_conf.c
@@ -148,10 +148,12 @@ virCHDriverConfigNew(bool privileged)
if (privileged) {
cfg->logDir = g_strdup_printf("%s/log/libvirt/ch", LOCALSTATEDIR);
cfg->stateDir = g_strdup_printf("%s/libvirt/ch", RUNSTATEDIR);
+cfg->saveDir = g_strdup_printf("%s/lib/libvirt/ch/save",
LOCALSTATEDIR);
} else {
g_autofree char *rundir = NULL;
g_autofree char *cachedir = NULL;
+g_autofree char *configbasedir = NULL;
cachedir = virGetUserCacheDirectory();
@@ -159,6 +161,9 @@ virCHDriverConfigNew(bool privileged)
rundir = virGetUserRuntimeDirectory();
cfg->stateDir = g_strdup_printf("%s/ch/run", rundir);
+
+configbasedir = virGetUserConfigDirectory();
+cfg->saveDir = g_strdup_printf("%s/ch/save", configbasedir);
}
return cfg;
@@ -175,6 +180,7 @@ virCHDriverConfigDispose(void *obj)
{
virCHDriverConfig *cfg = obj;
+g_free(cfg->saveDir);
g_free(cfg->stateDir);
g_free(cfg->logDir);
}
diff --git a/src/ch/ch_conf.h b/src/ch/ch_conf.h
index 4b4c3345b6..a77cad7a2a 100644
--- a/src/ch/ch_conf.h
+++ b/src/ch/ch_conf.h
@@ -37,6 +37,7 @@ struct _virCHDriverConfig {
char *stateDir;
char *logDir;
+char *saveDir;
int cgroupControllers;
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 24f697d2e1..be6e25c8e2 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -178,6 +178,14 @@ static char *chConnectGetCapabilities(virConnectPtr conn)
return xml;
}
+static char *
+chDomainManagedSavePath(virCHDriver *driver, virDomainObj *vm)
+{
+g_autoptr(virCHDriverConfig) cfg = virCHDriverGetConfig(driver);
+return g_strdup_printf("%s/%s.save", cfg->saveDir, vm->def->name);
+}
+
+
/**
* chDomainCreateXML:
* @conn: pointer to connection
@@ -763,6 +771,48 @@ chDomainSave(virDomainPtr dom, const char *to)
return chDomainSaveFlags(dom, to, NULL, 0);
}
+static int
+chDomainManagedSave(virDomainPtr dom, unsigned int flags)
+{
+virCHDriver *driver = dom->conn->privateData;
+virDomainObj *vm = NULL;
+g_autofree char *to = NULL;
+int ret = -1;
+
+virCheckFlags(0, -1);
+
+if (!(vm = virCHDomainObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainManagedSaveEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0)
+goto cleanup;
+
+if (virDomainObjCheckActive(vm) < 0)
+goto endjob;
+
+if (!vm->persistent) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("cannot do managed save for transient domain"));
+goto endjob;
+}
+
+to = chDomainManagedSavePath(driver, vm);
+if (chDoDomainSave(driver, vm, to, true) < 0)
+goto endjob;
+
+ret = 0;
+
+ endjob:
+virDomainObjEndJob(vm);
+
+ cleanup:
+virDomainObjEndAPI(&vm);
+return ret;
+}
+
static virDomainPtr chDomainLookupByID(virConnectPtr conn,
int id)
{
@@ -1887,6 +1937,7 @@ static virHypervisorDriver chHypervisorDriver = {
.domainGetNumaParameters = chDomainGetNumaParameters, /* 8.1.0 */
.domainSave = chDomainSave, /* 10.2.0 */
.domainSaveFlags = chDomainSaveFlags, /* 10.2.0 */
+.domainManagedSave = chDomainManagedSave, /* 10.2.0 */
};
static virConnectDriver chConnectDriver = {
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index 939fa13667..7b6b77de1c 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -557,6 +557,13 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg)
return NULL;
}
+if (g_mkdir_with_parents(cfg->saveDir, 0777) < 0) {
+virReportSystemError(errno,
+ _("Cannot create save directory '%1$s'"),
+ cfg->saveDir);
+return NULL;
+}
+
cmd = virCommandNew(vm->def->emulator);
virCommandSetUmask(cmd, 0x002);
socket_fd = chMonitorCreateSocket(mon->socketpath);
--
2.34.1
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 05/11] ch_driver: Implement more save callbacks
Following callbacks have been implemented
* domainSaveImageGetXMLDesc
* domainManagedSaveRemove
* domainManagedSaveGetXMLDesc
* domainHasManagedSaveImage
Signed-off-by: Purna Pavan Chandra Aekkaladevi
---
src/ch/ch_driver.c | 180 +
1 file changed, 180 insertions(+)
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index be6e25c8e2..577544c941 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -771,6 +771,99 @@ chDomainSave(virDomainPtr dom, const char *to)
return chDomainSaveFlags(dom, to, NULL, 0);
}
+static char *
+chDomainSaveXMLRead(int fd)
+{
+g_autofree char *xml = NULL;
+CHSaveXMLHeader hdr;
+
+if (saferead(fd, &hdr, sizeof(hdr)) != sizeof(hdr)) {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("failed to read CHSaveXMLHeader header"));
+return NULL;
+}
+
+if (memcmp(hdr.magic, CH_SAVE_MAGIC, sizeof(hdr.magic))) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("save image magic is incorrect"));
+return NULL;
+}
+
+if (hdr.xmlLen <= 0) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+ _("invalid XML length: %1$d"), hdr.xmlLen);
+return NULL;
+}
+
+xml = g_new0(char, hdr.xmlLen);
+
+if (saferead(fd, xml, hdr.xmlLen) != hdr.xmlLen) {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("failed to read XML"));
+return NULL;
+}
+
+return g_steal_pointer(&xml);
+}
+
+static int chDomainSaveImageRead(virCHDriver *driver,
+ const char *path,
+ virDomainDef **ret_def)
+{
+g_autoptr(virCHDriverConfig) cfg = virCHDriverGetConfig(driver);
+g_autoptr(virDomainDef) def = NULL;
+g_autofree char *from = NULL;
+g_autofree char *xml = NULL;
+VIR_AUTOCLOSE fd = -1;
+int ret = -1;
+
+from = g_strdup_printf("%s/%s", path, CH_SAVE_XML);
+if ((fd = virFileOpenAs(from, O_RDONLY, 0, cfg->user, cfg->group, 0)) < 0)
{
+virReportSystemError(errno,
+ _("Failed to open domain save file '%1$s'"),
+ from);
+goto end;
+}
+
+if (!(xml = chDomainSaveXMLRead(fd)))
+goto end;
+
+if (!(def = virDomainDefParseString(xml, driver->xmlopt, NULL,
+VIR_DOMAIN_DEF_PARSE_INACTIVE |
+VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
+goto end;
+
+*ret_def = g_steal_pointer(&def);
+ret = 0;
+
+ end:
+return ret;
+}
+
+static char *
+chDomainSaveImageGetXMLDesc(virConnectPtr conn,
+const char *path,
+unsigned int flags)
+{
+virCHDriver *driver = conn->privateData;
+g_autoptr(virDomainDef) def = NULL;
+char *ret = NULL;
+
+virCheckFlags(VIR_DOMAIN_SAVE_IMAGE_XML_SECURE, NULL);
+
+if (chDomainSaveImageRead(driver, path, &def) < 0)
+goto cleanup;
+
+if (virDomainSaveImageGetXMLDescEnsureACL(conn, def) < 0)
+goto cleanup;
+
+ret = virDomainDefFormat(def, driver->xmlopt,
+ virDomainDefFormatConvertXMLFlags(flags));
+
+ cleanup:
+return ret;
+}
+
static int
chDomainManagedSave(virDomainPtr dom, unsigned int flags)
{
@@ -813,6 +906,89 @@ chDomainManagedSave(virDomainPtr dom, unsigned int flags)
return ret;
}
+static int
+chDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
+{
+virCHDriver *driver = dom->conn->privateData;
+virDomainObj *vm;
+int ret = -1;
+g_autofree char *path = NULL;
+
+virCheckFlags(0, -1);
+
+if (!(vm = virCHDomainObjFromDomain(dom)))
+return -1;
+
+if (virDomainManagedSaveRemoveEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+path = chDomainManagedSavePath(driver, vm);
+
+if (virFileDeleteTree(path) < 0) {
+virReportSystemError(errno,
+ _("Failed to remove managed save path '%1$s'"),
+ path);
+goto cleanup;
+}
+
+vm->hasManagedSave = false;
+ret = 0;
+
+ cleanup:
+virDomainObjEndAPI(&vm);
+return ret;
+}
+
+static char *
+chDomainManagedSaveGetXMLDesc(virDomainPtr dom, unsigned int flags)
+{
+virCHDriver *driver = dom->conn->privateData;
+virDomainObj *vm = NULL;
+g_autoptr(virDomainDef) def = NULL;
+char *ret = NULL;
+g_autofree char *path = NULL;
+
+virCheckFlags(VIR_DOMAIN_SAVE_IMAGE_XML_SECURE, NULL);
+
+if (!(vm = virCHDomainObjFromDomain(dom)))
+goto cleanup;
+
+path = chDomainManagedSavePath(driver, vm);
+if (chDomainSaveImageRead(driver, path, &def) < 0)
+goto cleanup;
+
+if (virDomainManagedSaveGetXMLDescEnsureACL(dom->conn, def, flags) < 0)
+goto cleanup;
+
+ret = virDomainDefFormat(def, driver->xmlopt,
+
[PATCH 08/11] ch_driver: cleanup any stale managed save dir before VM creation
There are chances that libvirt process is killed and it resulting in
stale managed save dirs. So check for it, and cleanup it there's any.
Signed-off-by: Purna Pavan Chandra Aekkaladevi
---
src/ch/ch_driver.c | 20
1 file changed, 20 insertions(+)
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 218e2ec56f..09cd6b90e7 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -206,6 +206,7 @@ chDomainCreateXML(virConnectPtr conn,
virDomainObj *vm = NULL;
virDomainPtr dom = NULL;
unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE;
+g_autofree char *managed_save_path = NULL;
virCheckFlags(VIR_DOMAIN_START_VALIDATE, NULL);
@@ -228,6 +229,15 @@ chDomainCreateXML(virConnectPtr conn,
NULL)))
goto cleanup;
+/* cleanup if there's any stale managedsave dir */
+managed_save_path = chDomainManagedSavePath(driver, vm);
+if (virFileDeleteTree(managed_save_path) < 0) {
+virReportSystemError(errno,
+ _("Failed to cleanup stale managed save dir
'%1$s'"),
+ managed_save_path);
+goto cleanup;
+}
+
if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0)
goto cleanup;
@@ -315,6 +325,7 @@ chDomainDefineXMLFlags(virConnectPtr conn, const char *xml,
unsigned int flags)
g_autoptr(virDomainDef) vmdef = NULL;
virDomainObj *vm = NULL;
virDomainPtr dom = NULL;
+g_autofree char *managed_save_path = NULL;
unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE;
virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL);
@@ -337,6 +348,15 @@ chDomainDefineXMLFlags(virConnectPtr conn, const char
*xml, unsigned int flags)
0, NULL)))
goto cleanup;
+/* cleanup if there's any stale managedsave dir */
+managed_save_path = chDomainManagedSavePath(driver, vm);
+if (virFileDeleteTree(managed_save_path) < 0) {
+virReportSystemError(errno,
+ _("Failed to cleanup stale managed save dir
'%1$s'"),
+ managed_save_path);
+goto cleanup;
+}
+
vm->persistent = 1;
dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
--
2.34.1
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 06/11] ch_driver: Refactor virCHProcessStart
Signed-off-by: Purna Pavan Chandra Aekkaladevi
---
src/ch/ch_process.c | 47 -
1 file changed, 29 insertions(+), 18 deletions(-)
diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c
index 4b360413fb..02749adfb6 100644
--- a/src/ch/ch_process.c
+++ b/src/ch/ch_process.c
@@ -460,6 +460,34 @@ virCHProcessSetupVcpus(virDomainObj *vm)
return 0;
}
+static int
+virCHProcessSetup(virDomainObj *vm)
+{
+virCHDomainObjPrivate *priv = vm->privateData;
+
+virCHDomainRefreshThreadInfo(vm);
+
+VIR_DEBUG("Setting emulator tuning/settings");
+if (virCHProcessSetupEmulatorThreads(vm) < 0)
+return -1;
+
+VIR_DEBUG("Setting iothread tuning/settings");
+if (virCHProcessSetupIOThreads(vm) < 0)
+return -1;
+
+VIR_DEBUG("Setting global CPU cgroup (if required)");
+if (virDomainCgroupSetupGlobalCpuCgroup(vm,
+priv->cgroup) < 0)
+return -1;
+
+VIR_DEBUG("Setting vCPU tuning/settings");
+if (virCHProcessSetupVcpus(vm) < 0)
+return -1;
+
+virCHProcessUpdateInfo(vm);
+return 0;
+}
+
#define PKT_TIMEOUT_MS 500 /* ms */
@@ -763,26 +791,9 @@ virCHProcessStart(virCHDriver *driver,
goto cleanup;
}
-virCHDomainRefreshThreadInfo(vm);
-
-VIR_DEBUG("Setting emulator tuning/settings");
-if (virCHProcessSetupEmulatorThreads(vm) < 0)
-goto cleanup;
-
-VIR_DEBUG("Setting iothread tuning/settings");
-if (virCHProcessSetupIOThreads(vm) < 0)
-goto cleanup;
-
-VIR_DEBUG("Setting global CPU cgroup (if required)");
-if (virDomainCgroupSetupGlobalCpuCgroup(vm,
-priv->cgroup) < 0)
+if (virCHProcessSetup(vm) < 0)
goto cleanup;
-VIR_DEBUG("Setting vCPU tuning/settings");
-if (virCHProcessSetupVcpus(vm) < 0)
-goto cleanup;
-
-virCHProcessUpdateInfo(vm);
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, reason);
return 0;
--
2.34.1
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 07/11] ch_driver: Implement domain restore callbacks
Following callbacks have been implemented
* domainRestore
* domainRestoreFlags
The path parameter to these callbacks has to be of the directory where
libvirt has performed save. Additionally, call restore in `domainCreate`
if the domain has managedsave.
Signed-off-by: Purna Pavan Chandra Aekkaladevi
---
src/ch/ch_driver.c | 96 -
src/ch/ch_process.c | 53 +
src/ch/ch_process.h | 4 ++
3 files changed, 152 insertions(+), 1 deletion(-)
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 577544c941..218e2ec56f 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -252,6 +252,8 @@ chDomainCreateWithFlags(virDomainPtr dom, unsigned int
flags)
{
virCHDriver *driver = dom->conn->privateData;
virDomainObj *vm;
+virCHDomainObjPrivate *priv;
+g_autofree char *managed_save_path = NULL;
int ret = -1;
virCheckFlags(0, -1);
@@ -265,8 +267,34 @@ chDomainCreateWithFlags(virDomainPtr dom, unsigned int
flags)
if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0)
goto cleanup;
-ret = virCHProcessStart(driver, vm, VIR_DOMAIN_RUNNING_BOOTED);
+if (vm->hasManagedSave) {
+priv = vm->privateData;
+managed_save_path = chDomainManagedSavePath(driver, vm);
+if (virCHProcessStartRestore(driver, vm, managed_save_path) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to restore domain from managed save"));
+goto endjob;
+}
+if (virCHMonitorResumeVM(priv->monitor) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to resume domain after restore from
managed save"));
+goto endjob;
+}
+virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
VIR_DOMAIN_RUNNING_RESTORED);
+/* cleanup the save dir after restore */
+if (virFileDeleteTree(managed_save_path) < 0) {
+virReportSystemError(errno,
+ _("Failed to remove managed save path
'%1$s'"),
+ managed_save_path);
+goto endjob;
+}
+vm->hasManagedSave = false;
+ret = 0;
+} else {
+ret = virCHProcessStart(driver, vm, VIR_DOMAIN_RUNNING_BOOTED);
+}
+ endjob:
virDomainObjEndJob(vm);
cleanup:
@@ -989,6 +1017,70 @@ chDomainHasManagedSaveImage(virDomainPtr dom, unsigned
int flags)
return ret;
}
+static int
+chDomainRestoreFlags(virConnectPtr conn,
+ const char *from,
+ const char *dxml,
+ unsigned int flags)
+{
+virCHDriver *driver = conn->privateData;
+virDomainObj *vm = NULL;
+virCHDomainObjPrivate *priv;
+g_autoptr(virDomainDef) def = NULL;
+int ret = -1;
+
+virCheckFlags(0, -1);
+
+if (dxml) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("xml modification unsupported"));
+return -1;
+}
+
+if (chDomainSaveImageRead(driver, from, &def) < 0)
+goto cleanup;
+
+if (virDomainRestoreFlagsEnsureACL(conn, def) < 0)
+goto cleanup;
+
+if (!(vm = virDomainObjListAdd(driver->domains, &def,
+ driver->xmlopt,
+ VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
+ VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
+ NULL)))
+goto cleanup;
+
+if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0)
+goto cleanup;
+
+if (virCHProcessStartRestore(driver, vm, from) < 0)
+goto endjob;
+
+priv = vm->privateData;
+if (virCHMonitorResumeVM(priv->monitor) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to resume domain after restore"));
+goto endjob;
+}
+virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_RESTORED);
+ret = 0;
+
+ endjob:
+virDomainObjEndJob(vm);
+
+ cleanup:
+if (vm && ret < 0)
+virCHDomainRemoveInactive(driver, vm);
+virDomainObjEndAPI(&vm);
+return ret;
+}
+
+static int
+chDomainRestore(virConnectPtr conn, const char *from)
+{
+return chDomainRestoreFlags(conn, from, NULL, 0);
+}
+
static virDomainPtr chDomainLookupByID(virConnectPtr conn,
int id)
{
@@ -2118,6 +2210,8 @@ static virHypervisorDriver chHypervisorDriver = {
.domainManagedSaveRemove = chDomainManagedSaveRemove, /* 10.2.0 */
.domainManagedSaveGetXMLDesc = chDomainManagedSaveGetXMLDesc, /* 10.2.0
*/
.domainHasManagedSaveImage = chDomainHasManagedSaveImage, /* 10.2.0 */
+.domainRestore = chDomainRestore, /* 10.2.0 */
+.domainRestoreFlags = chDomainRestoreFlags, /* 10.2.0 */
};
static virConnectDriver chConnectDriver = {
diff --git a/src/ch/ch_process.c b/src/ch
[PATCH 11/11] NEWS: Mention save & restore support for ch driver
Signed-off-by: Purna Pavan Chandra Aekkaladevi --- NEWS.rst | 5 + 1 file changed, 5 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 44b775b546..e25265e1b0 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -16,6 +16,11 @@ v10.2.0 (unreleased) * **Removed features** * **New features** + * ch: Basic save and restore support for ch driver + + The ch driver now supports basic save and restore operations. This is functional + on domains without any network, host device config defined. The `path` parameter + for save and restore should be a directory. * **Improvements** -- 2.34.1 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 10/11] docs: Update doc for virDomainSave and virDomainRestore
ch_driver expects path to be of a dir for save/restore. So, update the documentation at global API as well. Signed-off-by: Purna Pavan Chandra Aekkaladevi --- src/libvirt-domain.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 9e3c61b66a..7c6b93963c 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -852,11 +852,11 @@ virDomainPMWakeup(virDomainPtr dom, /** * virDomainSave: * @domain: a domain object - * @to: path for the output file + * @to: path for the output save file / directory * - * This method will suspend a domain and save its memory contents to - * a file on disk. After the call, if successful, the domain is not - * listed as running anymore (this ends the life of a transient domain). + * This method will suspend a domain and save its memory contents to a file or + * direcotry (based on the vmm) on disk. After the call, if successful,the domain + * is not listed as running anymore (this ends the life of a transient domain). * Use virDomainRestore() to restore a domain after saving. * * See virDomainSaveFlags() and virDomainSaveParams() for more control. @@ -1053,7 +1053,7 @@ virDomainSaveParams(virDomainPtr domain, /** * virDomainRestore: * @conn: pointer to the hypervisor connection - * @from: path to the input file + * @from: path to the input save file / directory * * This method will restore a domain saved to disk by virDomainSave(). * -- 2.34.1 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 01/11] ch_driver: Support Save, Restore VM actions from monitor
Implement folowing API calls from CH monitor
* vmm.snapshot -> to save a domain
* vmm.restore -> to restore saved domain
Signed-off-by: Purna Pavan Chandra Aekkaladevi
---
src/ch/ch_monitor.c | 87 +
src/ch/ch_monitor.h | 4 +++
2 files changed, 91 insertions(+)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index fefbf7e67a..facbff002e 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -457,6 +457,22 @@ virCHMonitorBuildVMJson(virCHDriver *driver, virDomainDef
*vmdef,
return 0;
}
+static int
+virCHMonitorBuildKeyValueStringJson(char **jsonstr,
+const char *key,
+const char *value)
+{
+g_autoptr(virJSONValue) content = virJSONValueNewObject();
+
+if (virJSONValueObjectAppendString(content, key, value) < 0)
+return -1;
+
+if (!(*jsonstr = virJSONValueToString(content, false)))
+return -1;
+
+return 0;
+}
+
static int
chMonitorCreateSocket(const char *socket_path)
{
@@ -896,6 +912,77 @@ virCHMonitorResumeVM(virCHMonitor *mon)
return virCHMonitorPutNoContent(mon, URL_VM_RESUME);
}
+static int
+virCHMonitorSaveRestoreVM(virCHMonitor *mon, const char *path, bool save)
+{
+g_autofree char *url = NULL;
+int responseCode = 0;
+int ret = -1;
+g_autofree char *payload = NULL;
+g_autofree char *path_url = NULL;
+struct curl_slist *headers = NULL;
+struct curl_data data = {0};
+
+if (save)
+url = g_strdup_printf("%s/%s", URL_ROOT, URL_VM_SAVE);
+else
+url = g_strdup_printf("%s/%s", URL_ROOT, URL_VM_RESTORE);
+
+headers = curl_slist_append(headers, "Accept: application/json");
+headers = curl_slist_append(headers, "Content-Type: application/json");
+
+path_url = g_strdup_printf("file://%s", path);
+if (save) {
+if (virCHMonitorBuildKeyValueStringJson(&payload, "destination_url",
path_url) != 0)
+return -1;
+} else {
+if (virCHMonitorBuildKeyValueStringJson(&payload, "source_url",
path_url) != 0)
+return -1;
+}
+
+VIR_WITH_OBJECT_LOCK_GUARD(mon) {
+/* reset all options of a libcurl session handle at first */
+curl_easy_reset(mon->handle);
+
+curl_easy_setopt(mon->handle, CURLOPT_UNIX_SOCKET_PATH,
mon->socketpath);
+curl_easy_setopt(mon->handle, CURLOPT_URL, url);
+curl_easy_setopt(mon->handle, CURLOPT_CUSTOMREQUEST, "PUT");
+curl_easy_setopt(mon->handle, CURLOPT_HTTPHEADER, headers);
+curl_easy_setopt(mon->handle, CURLOPT_POSTFIELDS, payload);
+curl_easy_setopt(mon->handle, CURLOPT_WRITEFUNCTION, curl_callback);
+curl_easy_setopt(mon->handle, CURLOPT_WRITEDATA, (void *)&data);
+
+responseCode = virCHMonitorCurlPerform(mon->handle);
+}
+
+if (responseCode == 200 || responseCode == 204) {
+ret = 0;
+} else {
+data.content = g_realloc(data.content, data.size + 1);
+data.content[data.size] = 0;
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ data.content);
+g_free(data.content);
+}
+
+/* reset the libcurl handle to avoid leaking a stack pointer to data */
+curl_easy_reset(mon->handle);
+curl_slist_free_all(headers);
+return ret;
+}
+
+int
+virCHMonitorSaveVM(virCHMonitor *mon, const char *to)
+{
+return virCHMonitorSaveRestoreVM(mon, to, true);
+}
+
+int
+virCHMonitorRestoreVM(virCHMonitor *mon, const char *from)
+{
+return virCHMonitorSaveRestoreVM(mon, from, false);
+}
+
/**
* virCHMonitorGetInfo:
* @mon: Pointer to the monitor
diff --git a/src/ch/ch_monitor.h b/src/ch/ch_monitor.h
index 47b4e7abbd..3e0befe5d0 100644
--- a/src/ch/ch_monitor.h
+++ b/src/ch/ch_monitor.h
@@ -37,6 +37,8 @@
#define URL_VM_Suspend "vm.pause"
#define URL_VM_RESUME "vm.resume"
#define URL_VM_INFO "vm.info"
+#define URL_VM_SAVE "vm.snapshot"
+#define URL_VM_RESTORE "vm.restore"
#define VIRCH_THREAD_NAME_LEN 16
@@ -110,6 +112,8 @@ int virCHMonitorShutdownVM(virCHMonitor *mon);
int virCHMonitorRebootVM(virCHMonitor *mon);
int virCHMonitorSuspendVM(virCHMonitor *mon);
int virCHMonitorResumeVM(virCHMonitor *mon);
+int virCHMonitorSaveVM(virCHMonitor *mon, const char *to);
+int virCHMonitorRestoreVM(virCHMonitor *mon, const char *from);
int virCHMonitorGetInfo(virCHMonitor *mon, virJSONValue **info);
void virCHMonitorCPUInfoFree(virCHMonitorCPUInfo *cpus);
--
2.34.1
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 09/11] ch_driver: Add additional validation for save/restore
Save & Restore are supported without any network and hostdev config
defined. So, add a validation for it before performing save.
Signed-off-by: Purna Pavan Chandra Aekkaladevi
---
src/ch/ch_driver.c | 26 +-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 09cd6b90e7..3d5b8d211a 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -679,6 +679,26 @@ chDomainDestroy(virDomainPtr dom)
return chDomainDestroyFlags(dom, 0);
}
+static int
+chDomainSaveAdditionalValidation(virDomainDef *vmdef)
+{
+/*
+SAVE and RESTORE are functional only without any networking and
+device passthrough configuration
+*/
+if (vmdef->nnets > 0) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("cannot save domain with network interfaces"));
+return -1;
+}
+if (vmdef->nhostdevs > 0) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("cannot save domain with host devices"));
+return -1;
+}
+return 0;
+}
+
/**
* chDoDomainSave:
* @driver: pointer to driver structure
@@ -701,13 +721,17 @@ chDoDomainSave(virCHDriver *driver,
g_autoptr(virCHDriverConfig) cfg = virCHDriverGetConfig(driver);
virCHDomainObjPrivate *priv = vm->privateData;
CHSaveXMLHeader hdr;
+virDomainState domainState;
g_autofree char *to = NULL;
g_autofree char *xml = NULL;
uint32_t xml_len;
VIR_AUTOCLOSE fd = -1;
int ret = -1;
-virDomainState domainState = virDomainObjGetState(vm, NULL);
+if (chDomainSaveAdditionalValidation(vm->def) < 0)
+goto end;
+
+domainState = virDomainObjGetState(vm, NULL);
if (domainState == VIR_DOMAIN_RUNNING) {
if (virCHMonitorSuspendVM(priv->monitor) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
--
2.34.1
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 00/23] vsh: Fix handling of commands and help - part 1 (help text formatter and flags)
This series refactors the cruft in the virsh command and argument handling so that handling of options and the flags for them make actually sense. Except for the 'timeout' parameter of 'daemon-timeout' 'virt-admin' command the behaviour is supposed to be identical. Peter Krempa (23): vsh: Always assume that command groups are used vsh: Don't translate error messages for 'self-test' vsh: Add VSH_OT_NONE option type to catch programming errors vsh: Remove VSH_CMD_FLAG_ALIAS vshCmddefCheckInternals: Fix listing of missing completers for 'VSH_OT_ARGV' virsh: Remove uncommon redundant descriptions of virsh commands virt-admin: Drop trailing whitespaces from description of some commands virsh-domain: Don't explicitly break lines in help vsh: Add '--dump-help' option for 'self-test' command vsh: Refactor store of command help and description vshCmddefHelp: Refactor printing of help (list of arguments) vshCmddefHelp: Refactor printing of help (argument description) vshCmddefGetOption: Improve readability virsh: Inline only use of VIRSH_COMMON_OPT_DOMAIN_OT_ARGV macro vsh: Annotate 'required' and 'positional' arguments explicitly vsh: Fix broken assumption that required VSH_OT_INT must be positional vsh: Require that positional non-argv arguments are required vshCmddefCheckInternals: Remove refactoring safety checks vshCmdGrpHelp: Refactor formatting of help for VSH_OT_ARGV vshCmddefHelp: Refactor and fix printing of help for _STRING/_INT arguments vsh: Replace VSH_OT_DATA by VSH_OT_STRING vsh: remove VSH_OFLAG_REQ vsh: Automatically calculate VSH_OFLAG_REQ_OPT tools/virsh-backup.c | 29 +- tools/virsh-checkpoint.c | 109 +-- tools/virsh-domain-event.c | 13 +- tools/virsh-domain-monitor.c | 207 ++-- tools/virsh-domain.c | 1783 -- tools/virsh-host.c | 311 ++ tools/virsh-interface.c | 217 ++--- tools/virsh-network.c| 312 ++ tools/virsh-nodedev.c| 242 ++--- tools/virsh-nwfilter.c | 144 +-- tools/virsh-pool.c | 296 ++ tools/virsh-secret.c | 113 +-- tools/virsh-snapshot.c | 137 +-- tools/virsh-volume.c | 241 ++--- tools/virsh.c| 22 +- tools/virsh.h| 29 +- tools/virt-admin.c | 275 ++ tools/vsh.c | 474 + tools/vsh.h | 49 +- 19 files changed, 1798 insertions(+), 3205 deletions(-) -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 02/23] vsh: Don't translate error messages for 'self-test'
The command invoking the code is internal and meant for developers,
there's no point in translating the errors.
Signed-off-by: Peter Krempa
---
tools/vsh.c | 51 ++-
1 file changed, 26 insertions(+), 25 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index 0c41e9cfe8..61e302f9c8 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -255,7 +255,8 @@ vshCmddefGetInfo(const vshCmdDef * cmd, const char *name)
return NULL;
}
-/* Check if the internal command definitions are correct */
+/* Check if the internal command definitions are correct.
+ * None of the errors are to be marked as translatable. */
static int
vshCmddefCheckInternals(vshControl *ctl,
const vshCmdDef *cmd,
@@ -271,39 +272,39 @@ vshCmddefCheckInternals(vshControl *ctl,
const vshCmdDef *alias;
if (!cmd->alias) {
-vshError(ctl, _("command '%1$s' has inconsistent alias"),
cmd->name);
+vshError(ctl, "command '%s' has inconsistent alias", cmd->name);
return -1;
}
if (!(alias = vshCmddefSearch(cmd->alias))) {
-vshError(ctl, _("command alias '%1$s' is pointing to a
non-existent command '%2$s'"),
+vshError(ctl, "command alias '%s' is pointing to a non-existent
command '%s'",
cmd->name, cmd->alias);
return -1;
}
if (alias->flags & VSH_CMD_FLAG_ALIAS) {
-vshError(ctl, _("command alias '%1$s' is pointing to another
command alias '%2$s'"),
+vshError(ctl, "command alias '%s' is pointing to another command
alias '%s'",
cmd->name, cmd->alias);
return -1;
}
if (cmd->handler) {
-vshError(ctl, _("command '%1$s' has handler set"), cmd->name);
+vshError(ctl, "command '%s' has handler set", cmd->name);
return -1;
}
if (cmd->opts) {
-vshError(ctl, _("command '%1$s' has options set"), cmd->name);
+vshError(ctl, "command '%s' has options set", cmd->name);
return -1;
}
if (cmd->info) {
-vshError(ctl, _("command '%1$s' has info set"), cmd->name);
+vshError(ctl, "command '%s' has info set", cmd->name);
return -1;
}
if (cmd->flags & ~VSH_CMD_FLAG_ALIAS) {
-vshError(ctl, _("command '%1$s' has multiple flags set"),
cmd->name);
+vshError(ctl, "command '%s' has multiple flags set", cmd->name);
return -1;
}
@@ -313,7 +314,7 @@ vshCmddefCheckInternals(vshControl *ctl,
/* Each command has to provide a non-empty help string. */
if (!(help = vshCmddefGetInfo(cmd, "help")) || !*help) {
-vshError(ctl, _("command '%1$s' lacks help"), cmd->name);
+vshError(ctl, "command '%s' lacks help", cmd->name);
return -1;
}
@@ -324,7 +325,7 @@ vshCmddefCheckInternals(vshControl *ctl,
const vshCmdOptDef *opt = &cmd->opts[i];
if (i > 63) {
-vshError(ctl, _("command '%1$s' has too many options"), cmd->name);
+vshError(ctl, "command '%s' has too many options", cmd->name);
return -1; /* too many options */
}
@@ -336,7 +337,7 @@ vshCmddefCheckInternals(vshControl *ctl,
switch (opt->type) {
case VSH_OT_BOOL:
if (opt->completer || opt->completer_flags) {
-vshError(ctl, _("bool parameter '%1$s' of command '%2$s' has
completer set"),
+vshError(ctl, "bool parameter '%s' of command '%s' has
completer set",
opt->name, cmd->name);
return -1;
}
@@ -345,7 +346,7 @@ vshCmddefCheckInternals(vshControl *ctl,
case VSH_OT_STRING:
if (opt->flags & VSH_OFLAG_REQ) {
-vshError(ctl, _("parameter '%1$s' of command '%2$s' misused
VSH_OFLAG_REQ"),
+vshError(ctl, "parameter '%s' of command '%s' misused
VSH_OFLAG_REQ",
opt->name, cmd->name);
return -1; /* neither bool nor string options can be mandatory
*/
}
@@ -359,9 +360,9 @@ vshCmddefCheckInternals(vshControl *ctl,
char *p;
if (opt->flags || !opt->help) {
-vshError(ctl, _("parameter '%1$s' of command '%2$s' has
incorrect alias option"),
+vshError(ctl, "parameter '%s' of command '%s' has incorrect
alias option",
opt->name, cmd->name);
-return -1; /* alias options are tracked by the original name */
+return -1;
}
if ((p = strchr(opt->help, '=')))
name = g_strndup(opt->help, p - opt->help);
@@ -375,46 +376,46 @@ vshCmddefCheckInternals(vshControl *ctl,
if (p) {
/* If alias comes with value, replacement must not be bool */
[PATCH 01/23] vsh: Always assume that command groups are used
None of the clients use the 'command set' approach and other pieces of
code such as the command validator already assume that command groups
are in use. Remove the unused 'command set' stuff.
Signed-off-by: Peter Krempa
---
tools/virsh.c | 2 +-
tools/virt-admin.c | 2 +-
tools/vsh.c| 38 +++---
tools/vsh.h| 2 +-
4 files changed, 10 insertions(+), 34 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 95ff63baeb..18cd279aba 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -878,7 +878,7 @@ main(int argc, char **argv)
virFileActivateDirOverrideForProg(argv[0]);
-if (!vshInit(ctl, cmdGroups, NULL))
+if (!vshInit(ctl, cmdGroups))
exit(EXIT_FAILURE);
if (!virshParseArgv(ctl, argc, argv) ||
diff --git a/tools/virt-admin.c b/tools/virt-admin.c
index aaf6edb9a9..f551b33c4b 100644
--- a/tools/virt-admin.c
+++ b/tools/virt-admin.c
@@ -1594,7 +1594,7 @@ main(int argc, char **argv)
virFileActivateDirOverrideForProg(argv[0]);
-if (!vshInit(ctl, cmdGroups, NULL))
+if (!vshInit(ctl, cmdGroups))
exit(EXIT_FAILURE);
if (!vshAdmParseArgv(ctl, argc, argv) ||
diff --git a/tools/vsh.c b/tools/vsh.c
index 65deaa77e8..0c41e9cfe8 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -56,7 +56,6 @@ vshControl *autoCompleteOpaque;
* and only relies on static data accessible from the user-side callback
*/
const vshCmdGrp *cmdGroups;
-const vshCmdDef *cmdSet;
double
@@ -572,8 +571,8 @@ vshCommandCheckOpts(vshControl *ctl, const vshCmd *cmd,
uint64_t opts_required,
return -1;
}
-static const vshCmdDef *
-vshCmdDefSearchGrp(const char *cmdname)
+const vshCmdDef *
+vshCmddefSearch(const char *cmdname)
{
const vshCmdGrp *g;
const vshCmdDef *c;
@@ -588,28 +587,6 @@ vshCmdDefSearchGrp(const char *cmdname)
return NULL;
}
-static const vshCmdDef *
-vshCmdDefSearchSet(const char *cmdname)
-{
-const vshCmdDef *s;
-
-for (s = cmdSet; s->name; s++) {
-if (STREQ(s->name, cmdname))
-return s;
-}
-
-return NULL;
-}
-
-const vshCmdDef *
-vshCmddefSearch(const char *cmdname)
-{
-if (cmdGroups)
-return vshCmdDefSearchGrp(cmdname);
-else
-return vshCmdDefSearchSet(cmdname);
-}
-
const vshCmdGrp *
vshCmdGrpSearch(const char *grpname)
{
@@ -3012,20 +2989,19 @@ vshInitDebug(vshControl *ctl)
* Initialize global data
*/
bool
-vshInit(vshControl *ctl, const vshCmdGrp *groups, const vshCmdDef *set)
+vshInit(vshControl *ctl, const vshCmdGrp *groups)
{
if (!ctl->hooks) {
vshError(ctl, "%s", _("client hooks cannot be NULL"));
return false;
}
-if (!groups && !set) {
-vshError(ctl, "%s", _("command groups and command set cannot both be
NULL"));
+if (!groups) {
+vshError(ctl, "%s", _("command groups must be non-NULL"));
return false;
}
cmdGroups = groups;
-cmdSet = set;
if (vshInitDebug(ctl) < 0 ||
(ctl->imode && vshReadlineInit(ctl) < 0))
@@ -3037,8 +3013,8 @@ vshInit(vshControl *ctl, const vshCmdGrp *groups, const
vshCmdDef *set)
bool
vshInitReload(vshControl *ctl)
{
-if (!cmdGroups && !cmdSet) {
-vshError(ctl, "%s", _("command groups and command are both NULL run
vshInit before reloading"));
+if (!cmdGroups) {
+vshError(ctl, "%s", _("command groups is NULL run vshInit before
reloading"));
return false;
}
diff --git a/tools/vsh.h b/tools/vsh.h
index 2a1be29b1c..17d7f08dc9 100644
--- a/tools/vsh.h
+++ b/tools/vsh.h
@@ -310,7 +310,7 @@ void vshPrint(vshControl *ctl, const char *format, ...)
G_GNUC_PRINTF(2, 3);
void vshPrintExtra(vshControl *ctl, const char *format, ...)
G_GNUC_PRINTF(2, 3);
-bool vshInit(vshControl *ctl, const vshCmdGrp *groups, const vshCmdDef *set);
+bool vshInit(vshControl *ctl, const vshCmdGrp *groups);
bool vshInitReload(vshControl *ctl);
void vshDeinit(vshControl *ctl);
void vshDebug(vshControl *ctl, int level, const char *format, ...)
--
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 03/23] vsh: Add VSH_OT_NONE option type to catch programming errors
Add a check that the default 0 assignment will not mean that an option
is considered to be VSH_OT_BOOL.
Signed-off-by: Peter Krempa
---
tools/vsh.c | 7 +++
tools/vsh.h | 1 +
2 files changed, 8 insertions(+)
diff --git a/tools/vsh.c b/tools/vsh.c
index 61e302f9c8..a3491695b9 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -335,6 +335,11 @@ vshCmddefCheckInternals(vshControl *ctl,
virBufferStrcat(&complbuf, opt->name, ", ", NULL);
switch (opt->type) {
+case VSH_OT_NONE:
+vshError(ctl, "invalid type 'NONE' of option '%s' of command '%s'",
+ opt->name, cmd->name);
+return -1;
+
case VSH_OT_BOOL:
if (opt->completer || opt->completer_flags) {
vshError(ctl, "bool parameter '%s' of command '%s' has
completer set",
@@ -671,6 +676,7 @@ vshCmddefHelp(const vshCmdDef *def)
}
break;
case VSH_OT_ALIAS:
+case VSH_OT_NONE:
/* aliases are intentionally undocumented */
continue;
}
@@ -713,6 +719,7 @@ vshCmddefHelp(const vshCmdDef *def)
opt->name);
break;
case VSH_OT_ALIAS:
+case VSH_OT_NONE:
continue;
}
diff --git a/tools/vsh.h b/tools/vsh.h
index 17d7f08dc9..2cc0a62d3f 100644
--- a/tools/vsh.h
+++ b/tools/vsh.h
@@ -85,6 +85,7 @@ typedef enum {
* vshCmdOptType - command option type
*/
typedef enum {
+VSH_OT_NONE = 0, /* cannary to catch programming errors */
VSH_OT_BOOL, /* optional boolean option */
VSH_OT_STRING, /* optional string option */
VSH_OT_INT, /* optional or mandatory int option */
--
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 05/23] vshCmddefCheckInternals: Fix listing of missing completers for 'VSH_OT_ARGV'
Use a switch statement to cover all cases and check for missing
completers for arguments declared as VSH_OT_ARGV.
Signed-off-by: Peter Krempa
---
tools/vsh.c | 22 ++
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index 08a08bb227..6e558e56df 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -324,10 +324,24 @@ vshCmddefCheckInternals(vshControl *ctl,
return -1; /* too many options */
}
-if (missingCompleters &&
-(opt->type == VSH_OT_STRING || opt->type == VSH_OT_DATA) &&
-!opt->completer)
-virBufferStrcat(&complbuf, opt->name, ", ", NULL);
+if (missingCompleters && !opt->completer) {
+switch (opt->type) {
+case VSH_OT_STRING:
+case VSH_OT_DATA:
+case VSH_OT_ARGV:
+virBufferStrcat(&complbuf, opt->name, ", ", NULL);
+break;
+
+case VSH_OT_BOOL:
+/* only name is completed */
+case VSH_OT_INT:
+/* no point in completing numbers */
+case VSH_OT_ALIAS:
+/* alias is handled in the referenced command */
+case VSH_OT_NONE:
+break;
+}
+}
switch (opt->type) {
case VSH_OT_NONE:
--
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 06/23] virsh: Remove uncommon redundant descriptions of virsh commands
Some description of virsh commands referenced itself in a multi-line
example of usage, which is pointless as virsh help already shows how to
use the command:
.data = N_("Get or set the current memory parameters for a guest"
" domain.\n"
"To get the memory parameters use following command: \n\n"
"virsh # memtune ")
Change it to just state what the command does and leave the example for
the help printer.
Signed-off-by: Peter Krempa
---
tools/virsh-domain.c | 20
tools/virsh-host.c | 4 +---
2 files changed, 5 insertions(+), 19 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index d3e28f38f4..a90aefeb32 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1528,10 +1528,7 @@ static const vshCmdInfo info_blkiotune[] = {
.data = N_("Get or set blkio parameters")
},
{.name = "desc",
- .data = N_("Get or set the current blkio parameters for a guest"
-" domain.\n"
-"To get the blkio parameters use following command: \n\n"
-"virsh # blkiotune ")
+ .data = N_("Get or set the current blkio parameters for a guest domain.")
},
{.name = NULL}
};
@@ -9257,10 +9254,7 @@ static const vshCmdInfo info_memtune[] = {
.data = N_("Get or set memory parameters")
},
{.name = "desc",
- .data = N_("Get or set the current memory parameters for a guest"
-" domain.\n"
-"To get the memory parameters use following command: \n\n"
-"virsh # memtune ")
+ .data = N_("Get or set the current memory parameters for a guest domain.")
},
{.name = NULL}
};
@@ -9432,10 +9426,7 @@ static const vshCmdInfo info_perf[] = {
.data = N_("Get or set perf event")
},
{.name = "desc",
-.data = N_("Get or set the current perf events for a guest"
- " domain.\n"
- "To get the perf events list use following command:
\n\n"
- "virsh # perf ")
+.data = N_("Get or set the current perf events for a guest domain.")
},
{.name = NULL}
};
@@ -9564,10 +9555,7 @@ static const vshCmdInfo info_numatune[] = {
.data = N_("Get or set numa parameters")
},
{.name = "desc",
- .data = N_("Get or set the current numa parameters for a guest"
-" domain.\n"
-"To get the numa parameters use following command: \n\n"
-"virsh # numatune ")
+ .data = N_("Get or set the current numa parameters for a guest domain.")
},
{.name = NULL}
};
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 90ff46c5c4..87443220bd 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -1505,9 +1505,7 @@ cmdVersion(vshControl *ctl, const vshCmd *cmd
G_GNUC_UNUSED)
static const vshCmdInfo info_node_memory_tune[] = {
{"help", N_("Get or set node memory parameters")},
-{"desc", N_("Get or set node memory parameters\n"
-"To get the memory parameters, use following command: \n\n"
-"virsh # node-memory-tune")},
+{"desc", N_("Get or set node memory parameters")},
{NULL, NULL}
};
--
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 04/23] vsh: Remove VSH_CMD_FLAG_ALIAS
It's obvious that a command is an alias when the 'alias' property is
set, thus an extra flag is redundant. Remove it.
Signed-off-by: Peter Krempa
---
tools/virsh-nodedev.c | 1 -
tools/virsh.c | 2 +-
tools/virt-admin.c| 9 +
tools/vsh.c | 21 -
tools/vsh.h | 3 +--
5 files changed, 11 insertions(+), 25 deletions(-)
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index c08de65a96..b5a88b194f 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -1415,7 +1415,6 @@ const vshCmdDef nodedevCmds[] = {
.flags = 0
},
{.name = "nodedev-dettach",
- .flags = VSH_CMD_FLAG_ALIAS,
.alias = "nodedev-detach"
},
{.name = "nodedev-dumpxml",
diff --git a/tools/virsh.c b/tools/virsh.c
index 18cd279aba..0d860d0390 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -460,7 +460,7 @@ virshUsage(void)
fprintf(stdout, _(" %1$s (help keyword '%2$s')\n"),
grp->name, grp->keyword);
for (cmd = grp->commands; cmd->name; cmd++) {
-if (cmd->flags & VSH_CMD_FLAG_ALIAS ||
+if (cmd->alias ||
cmd->flags & VSH_CMD_FLAG_HIDDEN)
continue;
fprintf(stdout,
diff --git a/tools/virt-admin.c b/tools/virt-admin.c
index f551b33c4b..27bd123bfe 100644
--- a/tools/virt-admin.c
+++ b/tools/virt-admin.c
@@ -1262,7 +1262,7 @@ vshAdmUsage(void)
fprintf(stdout, _(" %1$s (help keyword '%2$s')\n"),
grp->name, grp->keyword);
for (cmd = grp->commands; cmd->name; cmd++) {
-if (cmd->flags & VSH_CMD_FLAG_ALIAS ||
+if (cmd->alias ||
cmd->flags & VSH_CMD_FLAG_HIDDEN)
continue;
fprintf(stdout,
@@ -1429,7 +1429,6 @@ static const vshCmdDef vshAdmCmds[] = {
static const vshCmdDef monitoringCmds[] = {
{.name = "srv-list",
- .flags = VSH_CMD_FLAG_ALIAS,
.alias = "server-list"
},
{.name = "server-list",
@@ -1439,7 +1438,6 @@ static const vshCmdDef monitoringCmds[] = {
.flags = 0
},
{.name = "srv-threadpool-info",
- .flags = VSH_CMD_FLAG_ALIAS,
.alias = "server-threadpool-info"
},
{.name = "server-threadpool-info",
@@ -1449,7 +1447,6 @@ static const vshCmdDef monitoringCmds[] = {
.flags = 0
},
{.name = "srv-clients-list",
- .flags = VSH_CMD_FLAG_ALIAS,
.alias = "client-list"
},
{.name = "client-list",
@@ -1465,7 +1462,6 @@ static const vshCmdDef monitoringCmds[] = {
.flags = 0
},
{.name = "srv-clients-info",
- .flags = VSH_CMD_FLAG_ALIAS,
.alias = "server-clients-info"
},
{.name = "server-clients-info",
@@ -1479,7 +1475,6 @@ static const vshCmdDef monitoringCmds[] = {
static const vshCmdDef managementCmds[] = {
{.name = "srv-threadpool-set",
- .flags = VSH_CMD_FLAG_ALIAS,
.alias = "server-threadpool-set"
},
{.name = "server-threadpool-set",
@@ -1495,7 +1490,6 @@ static const vshCmdDef managementCmds[] = {
.flags = 0
},
{.name = "srv-clients-set",
- .flags = VSH_CMD_FLAG_ALIAS,
.alias = "server-clients-set"
},
{.name = "server-clients-set",
@@ -1505,7 +1499,6 @@ static const vshCmdDef managementCmds[] = {
.flags = 0
},
{.name = "srv-update-tls",
- .flags = VSH_CMD_FLAG_ALIAS,
.alias = "server-update-tls"
},
{.name = "server-update-tls",
diff --git a/tools/vsh.c b/tools/vsh.c
index a3491695b9..08a08bb227 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -268,21 +268,16 @@ vshCmddefCheckInternals(vshControl *ctl,
g_auto(virBuffer) complbuf = VIR_BUFFER_INITIALIZER;
/* in order to perform the validation resolve the alias first */
-if (cmd->flags & VSH_CMD_FLAG_ALIAS) {
+if (cmd->alias) {
const vshCmdDef *alias;
-if (!cmd->alias) {
-vshError(ctl, "command '%s' has inconsistent alias", cmd->name);
-return -1;
-}
-
if (!(alias = vshCmddefSearch(cmd->alias))) {
vshError(ctl, "command alias '%s' is pointing to a non-existent
command '%s'",
cmd->name, cmd->alias);
return -1;
}
-if (alias->flags & VSH_CMD_FLAG_ALIAS) {
+if (alias->alias) {
vshError(ctl, "command alias '%s' is pointing to another command
alias '%s'",
cmd->name, cmd->alias);
return -1;
@@ -303,7 +298,7 @@ vshCmddefCheckInternals(vshControl *ctl,
return -1;
}
-if (cmd->flags & ~VSH_CMD_FLAG_ALIAS) {
+if (cmd->flags != 0) {
vshError(ctl, "command '%s' has multiple flags set", cmd->name);
return -1;
}
@@ -615,7 +610,7 @@ vshCmdGrpHelp(vshControl *ctl, const vshCmdGrp *grp)
grp->keyword);
for (cmd = grp->commands; cmd->name; cmd++) {
-if (cmd->flags & VSH_CMD_FLAG_ALIAS |
[PATCH 07/23] virt-admin: Drop trailing whitespaces from description of some commands
Signed-off-by: Peter Krempa
---
tools/virt-admin.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virt-admin.c b/tools/virt-admin.c
index 27bd123bfe..32e0c82f36 100644
--- a/tools/virt-admin.c
+++ b/tools/virt-admin.c
@@ -376,7 +376,7 @@ static const vshCmdInfo info_srv_threadpool_info[] = {
.data = N_("get server workerpool parameters")
},
{.name = "desc",
- .data = N_("Retrieve threadpool attributes from a server. ")
+ .data = N_("Retrieve threadpool attributes from a server.")
},
{.name = NULL}
};
@@ -784,7 +784,7 @@ static const vshCmdInfo info_srv_clients_info[] = {
.data = N_("get server's client-related configuration limits")
},
{.name = "desc",
- .data = N_("Retrieve server's client-related configuration limits ")
+ .data = N_("Retrieve server's client-related configuration limits")
},
{.name = NULL}
};
--
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 08/23] virsh-domain: Don't explicitly break lines in help
Signed-off-by: Peter Krempa
---
tools/virsh-domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index a90aefeb32..e2764efc4a 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9565,7 +9565,7 @@ static const vshCmdOptDef opts_numatune[] = {
{.name = "mode",
.type = VSH_OT_STRING,
.completer = virshDomainNumatuneModeCompleter,
- .help = N_("NUMA mode, one of strict, preferred and interleave \n"
+ .help = N_("NUMA mode, one of strict, preferred and interleave "
"or a number from the virDomainNumatuneMemMode enum")
},
{.name = "nodeset",
--
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 09/23] vsh: Add '--dump-help' option for 'self-test' command
The new option dumps the full help outputs for every command so that
it's possible to conveniently check that subsequent refactors will not
impact any of the external functionality.
No man page entry is needed as the command is internal/undocumented.
Signed-off-by: Peter Krempa
---
tools/vsh.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/tools/vsh.c b/tools/vsh.c
index 6e558e56df..55921a0759 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -3312,6 +3312,10 @@ const vshCmdOptDef opts_selftest[] = {
.type = VSH_OT_BOOL,
.help = N_("output the list of options which are missing completers")
},
+{.name = "dump-help",
+ .type = VSH_OT_BOOL,
+ .help = N_("output help for each command")
+},
{.name = NULL}
};
const vshCmdInfo info_selftest[] = {
@@ -3330,9 +3334,14 @@ cmdSelfTest(vshControl *ctl, const vshCmd *cmd)
const vshCmdGrp *grp;
const vshCmdDef *def;
bool completers = vshCommandOptBool(cmd, "completers-missing");
+bool dumphelp = vshCommandOptBool(cmd, "dump-help");
for (grp = cmdGroups; grp->name; grp++) {
for (def = grp->commands; def->name; def++) {
+
+if (dumphelp && !def->alias)
+vshCmddefHelp(def);
+
if (vshCmddefCheckInternals(ctl, def, completers) < 0)
return false;
}
--
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 11/23] vshCmddefHelp: Refactor printing of help (list of arguments)
Extract flag check to a separate variable and replace ternary operators
by normal conditions and directly output the text rather than using
extra variable to improve readability.
Signed-off-by: Peter Krempa
---
tools/vsh.c | 48
1 file changed, 32 insertions(+), 16 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index 688161beda..add50fe670 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -630,47 +630,63 @@ vshCmddefHelp(const vshCmdDef *def)
if (def->opts) {
const vshCmdOptDef *opt;
for (opt = def->opts; opt->name; opt++) {
-const char *fmt = "%s";
+bool required_option = opt->flags & VSH_OFLAG_REQ;
+
switch (opt->type) {
case VSH_OT_BOOL:
-fmt = "[--%s]";
+fprintf(stdout, " [--%s]", opt->name);
break;
+
case VSH_OT_INT:
-/* xgettext:c-format */
-fmt = ((opt->flags & VSH_OFLAG_REQ) ? "<%s>"
- : _("[--%1$s ]"));
+if (required_option) {
+fprintf(stdout, " <%s>", opt->name);
+} else {
+fprintf(stdout, _(" [--%1$s ]"), opt->name);
+}
+
if (!(opt->flags & VSH_OFLAG_REQ_OPT))
shortopt = true;
break;
+
case VSH_OT_STRING:
-/* xgettext:c-format */
-fmt = _("[--%1$s ]");
+fprintf(stdout, _(" [--%1$s ]"), opt->name);
+
if (!(opt->flags & VSH_OFLAG_REQ_OPT))
shortopt = true;
break;
+
case VSH_OT_DATA:
-fmt = ((opt->flags & VSH_OFLAG_REQ) ? "<%s>" : "[<%s>]");
+if (required_option) {
+fprintf(stdout, " <%s>", opt->name);
+} else {
+fprintf(stdout, " [<%s>]", opt->name);
+}
+
if (!(opt->flags & VSH_OFLAG_REQ_OPT))
shortopt = true;
break;
+
case VSH_OT_ARGV:
-/* xgettext:c-format */
if (shortopt) {
-fmt = (opt->flags & VSH_OFLAG_REQ)
-? _("{[--%1$s] }...")
-: _("[[--%1$s] ]...");
+if (required_option) {
+fprintf(stdout, _(" {[--%1$s] }..."),
opt->name);
+} else {
+fprintf(stdout, _(" [[--%1$s] ]..."),
opt->name);
+}
} else {
-fmt = (opt->flags & VSH_OFLAG_REQ) ? _("<%1$s>...")
-: _("[<%1$s>]...");
+if (required_option) {
+fprintf(stdout, " <%s>...", opt->name);
+} else {
+fprintf(stdout, " [<%s>]...", opt->name);
+}
}
break;
+
case VSH_OT_ALIAS:
case VSH_OT_NONE:
/* aliases are intentionally undocumented */
continue;
}
-fputc(' ', stdout);
-fprintf(stdout, fmt, opt->name);
}
}
fputc('\n', stdout);
--
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 12/23] vshCmddefHelp: Refactor printing of help (argument description)
Extract flag check to a separate variable and replace ternary operators
by normal conditions and use allocated buffer instead of a static one
to improve readability.
Signed-off-by: Peter Krempa
---
tools/vsh.c | 34 ++
1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index add50fe670..af5a576932 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -619,7 +619,6 @@ vshCmdGrpHelp(vshControl *ctl, const vshCmdGrp *grp)
static bool
vshCmddefHelp(const vshCmdDef *def)
{
-char buf[256];
bool shortopt = false; /* true if 'arg' works instead of '--opt arg' */
fputs(_(" NAME\n"), stdout);
@@ -701,33 +700,44 @@ vshCmddefHelp(const vshCmdDef *def)
const vshCmdOptDef *opt;
fputs(_("\n OPTIONS\n"), stdout);
for (opt = def->opts; opt->name; opt++) {
+bool required_option = opt->flags & VSH_OFLAG_REQ;
+g_autofree char *optstr = NULL;
+
switch (opt->type) {
case VSH_OT_BOOL:
-g_snprintf(buf, sizeof(buf), "--%s", opt->name);
+optstr = g_strdup_printf("--%s", opt->name);
break;
+
case VSH_OT_INT:
-g_snprintf(buf, sizeof(buf),
- (opt->flags & VSH_OFLAG_REQ) ? _("[--%1$s]
")
- : _("--%1$s "), opt->name);
+if (required_option) {
+optstr = g_strdup_printf(_("[--%1$s] "),
opt->name);
+} else {
+optstr = g_strdup_printf(_("--%1$s "), opt->name);
+}
break;
+
case VSH_OT_STRING:
-g_snprintf(buf, sizeof(buf), _("--%1$s "), opt->name);
+optstr = g_strdup_printf(_("--%1$s "), opt->name);
break;
+
case VSH_OT_DATA:
-g_snprintf(buf, sizeof(buf), _("[--%1$s] "),
- opt->name);
+optstr = g_strdup_printf(_("[--%1$s] "), opt->name);
break;
+
case VSH_OT_ARGV:
-g_snprintf(buf, sizeof(buf),
- shortopt ? _("[--%1$s] ") : _("<%1$s>"),
- opt->name);
+if (shortopt) {
+optstr = g_strdup_printf(_("[--%1$s] "),
opt->name);
+} else {
+optstr = g_strdup_printf("<%s>", opt->name);
+}
break;
+
case VSH_OT_ALIAS:
case VSH_OT_NONE:
continue;
}
-fprintf(stdout, "%-15s %s\n", buf, _(opt->help));
+fprintf(stdout, "%-15s %s\n", optstr, _(opt->help));
}
}
fputc('\n', stdout);
--
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 13/23] vshCmddefGetOption: Improve readability
Declare one argument per line, separate disticnt conditions by newline,
move some checks earlier.
Signed-off-by: Peter Krempa
---
tools/vsh.c | 69 ++---
1 file changed, 39 insertions(+), 30 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index af5a576932..de68f93a1f 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -468,9 +468,14 @@ static vshCmdOptDef helpopt = {
.type = VSH_OT_BOOL,
.help = N_("print help for this function")
};
+
static const vshCmdOptDef *
-vshCmddefGetOption(vshControl *ctl, const vshCmdDef *cmd, const char *name,
- uint64_t *opts_seen, size_t *opt_index, char **optstr,
+vshCmddefGetOption(vshControl *ctl,
+ const vshCmdDef *cmd,
+ const char *name,
+ uint64_t *opts_seen,
+ size_t *opt_index,
+ char **optstr,
bool report)
{
size_t i;
@@ -482,39 +487,43 @@ vshCmddefGetOption(vshControl *ctl, const vshCmdDef *cmd,
const char *name,
for (i = 0; cmd->opts && cmd->opts[i].name; i++) {
const vshCmdOptDef *opt = &cmd->opts[i];
-if (STREQ(opt->name, name)) {
-if (opt->type == VSH_OT_ALIAS) {
-char *value;
-
-/* Two types of replacements:
- opt->help = "string": straight replacement of name
- opt->help = "string=value": treat boolean flag as
- alias of option and its default value */
-alias = g_strdup(opt->help);
-name = alias;
-if ((value = strchr(name, '='))) {
-*value = '\0';
-if (*optstr) {
-if (report)
-vshError(ctl, _("invalid '=' after option --%1$s"),
- opt->name);
-return NULL;
-}
-*optstr = g_strdup(value + 1);
+if (STRNEQ(opt->name, name))
+continue;
+
+if (opt->type == VSH_OT_ALIAS) {
+char *value;
+
+/* Two types of replacements:
+ opt->help = "string": straight replacement of name
+ opt->help = "string=value": treat boolean flag as
+ alias of option and its default value */
+alias = g_strdup(opt->help);
+name = alias;
+if ((value = strchr(name, '='))) {
+*value = '\0';
+if (*optstr) {
+if (report)
+vshError(ctl, _("invalid '=' after option --%1$s"),
+ opt->name);
+return NULL;
}
-continue;
+*optstr = g_strdup(value + 1);
}
-if ((*opts_seen & (1ULL << i)) && opt->type != VSH_OT_ARGV) {
-if (report)
-vshError(ctl, _("option --%1$s already seen"), name);
-return NULL;
-}
-*opts_seen |= 1ULL << i;
-*opt_index = i;
-return opt;
+continue;
}
+
+if ((*opts_seen & (1ULL << i)) && opt->type != VSH_OT_ARGV) {
+if (report)
+vshError(ctl, _("option --%1$s already seen"), name);
+return NULL;
+}
+
+*opts_seen |= 1ULL << i;
+*opt_index = i;
+return opt;
}
+/* The 'help' command ignores extra options */
if (STRNEQ(cmd->name, "help") && report) {
vshError(ctl, _("command '%1$s' doesn't support option --%2$s"),
cmd->name, name);
--
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 14/23] virsh: Inline only use of VIRSH_COMMON_OPT_DOMAIN_OT_ARGV macro
There's just one command taking a list of domains as argument, thus
declare it inline.
Signed-off-by: Peter Krempa
---
tools/virsh-domain-monitor.c | 6 +-
tools/virsh.h| 12
2 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 277eb71342..7a25318bbd 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -2048,7 +2048,11 @@ static const vshCmdOptDef opts_domstats[] = {
.type = VSH_OT_BOOL,
.help = N_("report only stats that are accessible instantly"),
},
-VIRSH_COMMON_OPT_DOMAIN_OT_ARGV(N_("list of domains to get stats for"), 0),
+{.name = "domain",
+ .type = VSH_OT_ARGV,
+ .help = N_("list of domains to get stats for"),
+ .completer = virshDomainNameCompleter,
+},
{.name = NULL}
};
diff --git a/tools/virsh.h b/tools/virsh.h
index 6acefa7f9d..877b290e3a 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -119,18 +119,6 @@
VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("domain name, id or uuid"), \
oflags, cflags)
-#define VIRSH_COMMON_OPT_DOMAIN_OT_ARGV(_helpstr, cflags) \
-{.name = "domain", \
- .type = VSH_OT_ARGV, \
- .flags = VSH_OFLAG_NONE, \
- .help = _helpstr, \
- .completer = virshDomainNameCompleter, \
- .completer_flags = cflags, \
-}
-
-#define VIRSH_COMMON_OPT_DOMAIN_OT_ARGV_FULL(cflags) \
-VIRSH_COMMON_OPT_DOMAIN_OT_ARGV(N_("domain name, id or uuid"), cflags)
-
typedef struct _virshControl virshControl;
typedef struct _virshCtrlData virshCtrlData;
--
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 16/23] vsh: Fix broken assumption that required VSH_OT_INT must be positional
In at least one case we've wanted a mandatory argument which requires
the explicit flag. Fix the assumption before converting everything over
to the new flags.
Signed-off-by: Peter Krempa
---
tools/virt-admin.c | 1 -
tools/vsh.c| 9 +++--
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/tools/virt-admin.c b/tools/virt-admin.c
index d266d824cb..2e8895956d 100644
--- a/tools/virt-admin.c
+++ b/tools/virt-admin.c
@@ -1011,7 +1011,6 @@ static const vshCmdInfo info_daemon_log_outputs = {
static const vshCmdOptDef opts_daemon_timeout[] = {
{.name = "timeout",
.type = VSH_OT_INT,
- .positional = true,
.required = true,
.help = N_("number of seconds the daemon will run without any active
connection"),
.flags = VSH_OFLAG_REQ | VSH_OFLAG_REQ_OPT
diff --git a/tools/vsh.c b/tools/vsh.c
index 9745c122fe..7091cf093c 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -426,9 +426,14 @@ vshCmddefCheckInternals(vshControl *ctl,
opt->name, cmd->name);
return -1;
}
-seenPositionalOption = true;
-isPositional = true;
+
isRequired = true;
+
+/* allow INT arguments which are required and non-positional */
+if (!(opt->flags & VSH_OFLAG_REQ_OPT)) {
+seenPositionalOption = true;
+isPositional = true;
+}
} else {
isPositional = false;
isRequired = false;
--
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 15/23] vsh: Annotate 'required' and 'positional' arguments explicitly
Add 'positional' and 'required' fields to vshCmdOptDef, which will
explicitly track the two properties of arguments.
To ensure that we have proper coverage, add checks to
vshCmddefCheckInternals validating the state of the above flags by
infering it from existing data.
This conversion will allow us:
- remove VSH_OT_DATA in favor of VSH_OT_STRING
- use VSH_OT_INT when required both as positional and non-positional
- properly annotate which VSH_OT_ARGV are positional and which are not
(currently inferred by whether an previous positional option exists)
Signed-off-by: Peter Krempa
---
tools/virsh-domain-monitor.c | 5 ++
tools/virsh-domain.c | 118 +++
tools/virsh-host.c | 10 +++
tools/virsh-interface.c | 12
tools/virsh-network.c| 16 -
tools/virsh-nodedev.c| 20 ++
tools/virsh-nwfilter.c | 10 +++
tools/virsh-pool.c | 8 +++
tools/virsh-secret.c | 8 +++
tools/virsh-snapshot.c | 2 +
tools/virsh-volume.c | 12
tools/virsh.h| 16 +++--
tools/virt-admin.c | 22 +++
tools/vsh.c | 32 +-
tools/vsh.h | 2 +
15 files changed, 286 insertions(+), 7 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 7a25318bbd..573451c678 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -738,6 +738,8 @@ static const vshCmdOptDef opts_domif_getlink[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(0),
{.name = "interface",
.type = VSH_OT_DATA,
+ .positional = true,
+ .required = true,
.flags = VSH_OFLAG_REQ,
.completer = virshDomainInterfaceCompleter,
.help = N_("interface device (MAC Address)")
@@ -1038,6 +1040,8 @@ static const vshCmdOptDef opts_domifstat[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
{.name = "interface",
.type = VSH_OT_DATA,
+ .positional = true,
+ .required = true,
.flags = VSH_OFLAG_REQ,
.completer = virshDomainInterfaceCompleter,
.help = N_("interface device specified by name or MAC Address")
@@ -2050,6 +2054,7 @@ static const vshCmdOptDef opts_domstats[] = {
},
{.name = "domain",
.type = VSH_OT_ARGV,
+ .positional = true,
.help = N_("list of domains to get stats for"),
.completer = virshDomainNameCompleter,
},
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 6768a1f24e..7a63757131 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -420,11 +420,15 @@ static const vshCmdOptDef opts_attach_disk[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(0),
{.name = "source",
.type = VSH_OT_DATA,
+ .positional = true,
+ .required = true,
.flags = VSH_OFLAG_REQ | VSH_OFLAG_EMPTY_OK,
.help = N_("source of disk device or name of network disk")
},
{.name = "target",
.type = VSH_OT_DATA,
+ .positional = true,
+ .required = true,
.flags = VSH_OFLAG_REQ,
.completer = virshCompleteEmpty,
.help = N_("target of disk device")
@@ -812,11 +816,15 @@ static const vshCmdOptDef opts_attach_interface[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(0),
{.name = "type",
.type = VSH_OT_DATA,
+ .positional = true,
+ .required = true,
.flags = VSH_OFLAG_REQ,
.help = N_("network interface type")
},
{.name = "source",
.type = VSH_OT_DATA,
+ .positional = true,
+ .required = true,
.flags = VSH_OFLAG_REQ,
.help = N_("source of network interface")
},
@@ -1188,6 +1196,8 @@ static const vshCmdOptDef opts_blkdeviotune[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(0),
{.name = "device",
.type = VSH_OT_DATA,
+ .positional = true,
+ .required = true,
.flags = VSH_OFLAG_REQ,
.completer = virshDomainDiskTargetCompleter,
.help = N_("block device")
@@ -1981,6 +1991,8 @@ static const vshCmdOptDef opts_blockcommit[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
{.name = "path",
.type = VSH_OT_DATA,
+ .positional = true,
+ .required = true,
.flags = VSH_OFLAG_REQ,
.completer = virshDomainDiskTargetCompleter,
.help = N_("fully-qualified path of disk")
@@ -2200,6 +2212,8 @@ static const vshCmdOptDef opts_blockcopy[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
{.name = "path",
.type = VSH_OT_DATA,
+ .positional = true,
+ .required = true,
.flags = VSH_OFLAG_REQ,
.completer = virshDomainDiskTargetCompleter,
.help = N_("fully-qualified path of source disk")
@@ -2532,6 +2546,8 @@ static const vshCmdOptDef opts_blockjob[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
{.name = "path",
.type = VSH_OT_DATA,
+ .positional = true,
+ .required = true,
.flags = VSH_OFLAG_REQ,
.completer =
[PATCH 17/23] vsh: Require that positional non-argv arguments are required
This is logically enforced by existing checks, thus we can formalize it.
Signed-off-by: Peter Krempa
---
tools/vsh.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/tools/vsh.c b/tools/vsh.c
index 7091cf093c..301fa9d747 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -329,6 +329,13 @@ vshCmddefCheckInternals(vshControl *ctl,
}
}
+/* require that positional non-argv options are required */
+if (opt->positional && !opt->required && opt->type != VSH_OT_ARGV) {
+vshError(ctl, "positional argument '%s' of command '%s' must be
required",
+ opt->name, cmd->name);
+return -1;
+}
+
switch (opt->type) {
case VSH_OT_NONE:
vshError(ctl, "invalid type 'NONE' of option '%s' of command '%s'",
--
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 18/23] vshCmddefCheckInternals: Remove refactoring safety checks
Now that the code was refactored and proved identical, remove the checks
so that they don't impede further refactors.
Signed-off-by: Peter Krempa
---
tools/vsh.c | 34 --
1 file changed, 34 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index 301fa9d747..3245e64f5e 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -248,7 +248,6 @@ vshCmddefCheckInternals(vshControl *ctl,
{
size_t i;
bool seenOptionalOption = false;
-bool seenPositionalOption = false;
g_auto(virBuffer) complbuf = VIR_BUFFER_INITIALIZER;
/* in order to perform the validation resolve the alias first */
@@ -302,8 +301,6 @@ vshCmddefCheckInternals(vshControl *ctl,
for (i = 0; cmd->opts[i].name; i++) {
const vshCmdOptDef *opt = &cmd->opts[i];
-bool isPositional = false;
-bool isRequired = false;
if (i > 63) {
vshError(ctl, "command '%s' has too many options", cmd->name);
@@ -402,10 +399,6 @@ vshCmddefCheckInternals(vshControl *ctl,
opt->name, cmd->name);
return -1;
}
-
-isRequired = opt->flags & VSH_OFLAG_REQ;
-/* ARGV argument is positional if there are no positional options
*/
-isPositional = !seenPositionalOption;
break;
case VSH_OT_DATA:
@@ -415,10 +408,6 @@ vshCmddefCheckInternals(vshControl *ctl,
return -1;
}
-isRequired = true;
-isPositional = true;
-seenPositionalOption = true;
-
if (seenOptionalOption) {
vshError(ctl, "parameter '%s' of command '%s' must be listed
before optional parameters",
opt->name, cmd->name);
@@ -433,33 +422,10 @@ vshCmddefCheckInternals(vshControl *ctl,
opt->name, cmd->name);
return -1;
}
-
-isRequired = true;
-
-/* allow INT arguments which are required and non-positional */
-if (!(opt->flags & VSH_OFLAG_REQ_OPT)) {
-seenPositionalOption = true;
-isPositional = true;
-}
-} else {
-isPositional = false;
-isRequired = false;
}
break;
}
-
-if (opt->required != isRequired) {
-vshError(ctl, "parameter '%s' of command '%s' 'required' state
mismatch",
- opt->name, cmd->name);
-return -1;
-}
-
-if (opt->positional != isPositional) {
-vshError(ctl, "parameter '%s' of command '%s' 'positional' state
mismatch",
- opt->name, cmd->name);
-return -1;
-}
}
virBufferTrim(&complbuf, ", ");
--
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 19/23] vshCmdGrpHelp: Refactor formatting of help for VSH_OT_ARGV
Use the new properties rather than infer the states.
Signed-off-by: Peter Krempa
---
tools/vsh.c | 31 ++-
1 file changed, 10 insertions(+), 21 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index 3245e64f5e..521f222910 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -634,8 +634,6 @@ vshCmdGrpHelp(vshControl *ctl, const vshCmdGrp *grp)
static bool
vshCmddefHelp(const vshCmdDef *def)
{
-bool shortopt = false; /* true if 'arg' works instead of '--opt arg' */
-
fputs(_(" NAME\n"), stdout);
fprintf(stdout, "%s - %s\n", def->name, _(def->info->help));
@@ -657,16 +655,10 @@ vshCmddefHelp(const vshCmdDef *def)
} else {
fprintf(stdout, _(" [--%1$s ]"), opt->name);
}
-
-if (!(opt->flags & VSH_OFLAG_REQ_OPT))
-shortopt = true;
break;
case VSH_OT_STRING:
fprintf(stdout, _(" [--%1$s ]"), opt->name);
-
-if (!(opt->flags & VSH_OFLAG_REQ_OPT))
-shortopt = true;
break;
case VSH_OT_DATA:
@@ -675,23 +667,20 @@ vshCmddefHelp(const vshCmdDef *def)
} else {
fprintf(stdout, " [<%s>]", opt->name);
}
-
-if (!(opt->flags & VSH_OFLAG_REQ_OPT))
-shortopt = true;
break;
case VSH_OT_ARGV:
-if (shortopt) {
-if (required_option) {
-fprintf(stdout, _(" {[--%1$s] }..."),
opt->name);
+if (opt->positional) {
+if (opt->required) {
+fprintf(stdout, " <%s>...", opt->name);
} else {
-fprintf(stdout, _(" [[--%1$s] ]..."),
opt->name);
+fprintf(stdout, " [<%s>]...", opt->name);
}
} else {
-if (required_option) {
-fprintf(stdout, " <%s>...", opt->name);
+if (opt->required) {
+fprintf(stdout, _(" {[--%1$s] }..."),
opt->name);
} else {
-fprintf(stdout, " [<%s>]...", opt->name);
+fprintf(stdout, _(" [[--%1$s] ]..."),
opt->name);
}
}
break;
@@ -740,10 +729,10 @@ vshCmddefHelp(const vshCmdDef *def)
break;
case VSH_OT_ARGV:
-if (shortopt) {
-optstr = g_strdup_printf(_("[--%1$s] "),
opt->name);
-} else {
+if (opt->positional) {
optstr = g_strdup_printf("<%s>", opt->name);
+} else {
+optstr = g_strdup_printf(_("[--%1$s] "),
opt->name);
}
break;
--
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 21/23] vsh: Replace VSH_OT_DATA by VSH_OT_STRING
Use the new 'positional' field to do decisions rather than have a
special type for positional strings.
Signed-off-by: Peter Krempa
---
tools/virsh-domain-monitor.c | 4 +-
tools/virsh-domain.c | 94 ++--
tools/virsh-host.c | 4 +-
tools/virsh-interface.c | 12 ++---
tools/virsh-network.c| 14 +++---
tools/virsh-nodedev.c| 20
tools/virsh-nwfilter.c | 10 ++--
tools/virsh-pool.c | 8 +--
tools/virsh-secret.c | 8 +--
tools/virsh-snapshot.c | 2 +-
tools/virsh-volume.c | 12 ++---
tools/virsh.h| 6 +--
tools/virt-admin.c | 18 +++
tools/vsh.c | 33 +++--
tools/vsh.h | 1 -
15 files changed, 112 insertions(+), 134 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 573451c678..568ff770a1 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -737,7 +737,7 @@ static const vshCmdInfo info_domif_getlink = {
static const vshCmdOptDef opts_domif_getlink[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(0),
{.name = "interface",
- .type = VSH_OT_DATA,
+ .type = VSH_OT_STRING,
.positional = true,
.required = true,
.flags = VSH_OFLAG_REQ,
@@ -1039,7 +1039,7 @@ static const vshCmdInfo info_domifstat = {
static const vshCmdOptDef opts_domifstat[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
{.name = "interface",
- .type = VSH_OT_DATA,
+ .type = VSH_OT_STRING,
.positional = true,
.required = true,
.flags = VSH_OFLAG_REQ,
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 7a63757131..1dbef9a7a6 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -419,14 +419,14 @@ static const vshCmdInfo info_attach_disk = {
static const vshCmdOptDef opts_attach_disk[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(0),
{.name = "source",
- .type = VSH_OT_DATA,
+ .type = VSH_OT_STRING,
.positional = true,
.required = true,
.flags = VSH_OFLAG_REQ | VSH_OFLAG_EMPTY_OK,
.help = N_("source of disk device or name of network disk")
},
{.name = "target",
- .type = VSH_OT_DATA,
+ .type = VSH_OT_STRING,
.positional = true,
.required = true,
.flags = VSH_OFLAG_REQ,
@@ -815,14 +815,14 @@ static const vshCmdInfo info_attach_interface = {
static const vshCmdOptDef opts_attach_interface[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(0),
{.name = "type",
- .type = VSH_OT_DATA,
+ .type = VSH_OT_STRING,
.positional = true,
.required = true,
.flags = VSH_OFLAG_REQ,
.help = N_("network interface type")
},
{.name = "source",
- .type = VSH_OT_DATA,
+ .type = VSH_OT_STRING,
.positional = true,
.required = true,
.flags = VSH_OFLAG_REQ,
@@ -1195,7 +1195,7 @@ static const vshCmdInfo info_blkdeviotune = {
static const vshCmdOptDef opts_blkdeviotune[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(0),
{.name = "device",
- .type = VSH_OT_DATA,
+ .type = VSH_OT_STRING,
.positional = true,
.required = true,
.flags = VSH_OFLAG_REQ,
@@ -1990,7 +1990,7 @@ static const vshCmdInfo info_blockcommit = {
static const vshCmdOptDef opts_blockcommit[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
{.name = "path",
- .type = VSH_OT_DATA,
+ .type = VSH_OT_STRING,
.positional = true,
.required = true,
.flags = VSH_OFLAG_REQ,
@@ -2211,7 +2211,7 @@ static const vshCmdInfo info_blockcopy = {
static const vshCmdOptDef opts_blockcopy[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
{.name = "path",
- .type = VSH_OT_DATA,
+ .type = VSH_OT_STRING,
.positional = true,
.required = true,
.flags = VSH_OFLAG_REQ,
@@ -2545,7 +2545,7 @@ static const vshCmdInfo info_blockjob = {
static const vshCmdOptDef opts_blockjob[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
{.name = "path",
- .type = VSH_OT_DATA,
+ .type = VSH_OT_STRING,
.positional = true,
.required = true,
.flags = VSH_OFLAG_REQ,
@@ -2758,7 +2758,7 @@ static const vshCmdInfo info_blockpull = {
static const vshCmdOptDef opts_blockpull[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
{.name = "path",
- .type = VSH_OT_DATA,
+ .type = VSH_OT_STRING,
.positional = true,
.required = true,
.flags = VSH_OFLAG_REQ,
@@ -2902,7 +2902,7 @@ static const vshCmdInfo info_blockresize = {
static const vshCmdOptDef opts_blockresize[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
{.name = "path",
- .type = VSH_OT_DATA,
+ .type = VSH_OT_STRING,
.positional = true,
.required = true,
.flags = VSH_OFLAG_REQ,
@@ -3061,7 +3061,7 @@ static const vshCmdInfo info_dom
[PATCH 22/23] vsh: remove VSH_OFLAG_REQ
The flag was replaced by the 'required' field in the option definition.
Remove last few uses and all assignments.
Signed-off-by: Peter Krempa
---
tools/virsh-domain-monitor.c | 2 --
tools/virsh-domain.c | 62 +---
tools/virsh-host.c | 5 ---
tools/virsh-interface.c | 6
tools/virsh-network.c| 7
tools/virsh-nodedev.c| 10 --
tools/virsh-nwfilter.c | 5 ---
tools/virsh-pool.c | 4 ---
tools/virsh-secret.c | 4 ---
tools/virsh-snapshot.c | 1 -
tools/virsh-volume.c | 6
tools/virsh.h| 3 --
tools/virt-admin.c | 12 +--
tools/vsh.c | 8 ++---
tools/vsh.h | 1 -
15 files changed, 6 insertions(+), 130 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 568ff770a1..6c2499fb9f 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -740,7 +740,6 @@ static const vshCmdOptDef opts_domif_getlink[] = {
.type = VSH_OT_STRING,
.positional = true,
.required = true,
- .flags = VSH_OFLAG_REQ,
.completer = virshDomainInterfaceCompleter,
.help = N_("interface device (MAC Address)")
},
@@ -1042,7 +1041,6 @@ static const vshCmdOptDef opts_domifstat[] = {
.type = VSH_OT_STRING,
.positional = true,
.required = true,
- .flags = VSH_OFLAG_REQ,
.completer = virshDomainInterfaceCompleter,
.help = N_("interface device specified by name or MAC Address")
},
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 1dbef9a7a6..7c2613c85b 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -422,14 +422,13 @@ static const vshCmdOptDef opts_attach_disk[] = {
.type = VSH_OT_STRING,
.positional = true,
.required = true,
- .flags = VSH_OFLAG_REQ | VSH_OFLAG_EMPTY_OK,
+ .flags = VSH_OFLAG_EMPTY_OK,
.help = N_("source of disk device or name of network disk")
},
{.name = "target",
.type = VSH_OT_STRING,
.positional = true,
.required = true,
- .flags = VSH_OFLAG_REQ,
.completer = virshCompleteEmpty,
.help = N_("target of disk device")
},
@@ -818,14 +817,12 @@ static const vshCmdOptDef opts_attach_interface[] = {
.type = VSH_OT_STRING,
.positional = true,
.required = true,
- .flags = VSH_OFLAG_REQ,
.help = N_("network interface type")
},
{.name = "source",
.type = VSH_OT_STRING,
.positional = true,
.required = true,
- .flags = VSH_OFLAG_REQ,
.help = N_("source of network interface")
},
{.name = "target",
@@ -1198,7 +1195,6 @@ static const vshCmdOptDef opts_blkdeviotune[] = {
.type = VSH_OT_STRING,
.positional = true,
.required = true,
- .flags = VSH_OFLAG_REQ,
.completer = virshDomainDiskTargetCompleter,
.help = N_("block device")
},
@@ -1993,7 +1989,6 @@ static const vshCmdOptDef opts_blockcommit[] = {
.type = VSH_OT_STRING,
.positional = true,
.required = true,
- .flags = VSH_OFLAG_REQ,
.completer = virshDomainDiskTargetCompleter,
.help = N_("fully-qualified path of disk")
},
@@ -2214,7 +2209,6 @@ static const vshCmdOptDef opts_blockcopy[] = {
.type = VSH_OT_STRING,
.positional = true,
.required = true,
- .flags = VSH_OFLAG_REQ,
.completer = virshDomainDiskTargetCompleter,
.help = N_("fully-qualified path of source disk")
},
@@ -2548,7 +2542,6 @@ static const vshCmdOptDef opts_blockjob[] = {
.type = VSH_OT_STRING,
.positional = true,
.required = true,
- .flags = VSH_OFLAG_REQ,
.completer = virshDomainDiskTargetCompleter,
.help = N_("fully-qualified path of disk")
},
@@ -2761,7 +2754,6 @@ static const vshCmdOptDef opts_blockpull[] = {
.type = VSH_OT_STRING,
.positional = true,
.required = true,
- .flags = VSH_OFLAG_REQ,
.completer = virshDomainDiskTargetCompleter,
.help = N_("fully-qualified path of disk")
},
@@ -2905,7 +2897,6 @@ static const vshCmdOptDef opts_blockresize[] = {
.type = VSH_OT_STRING,
.positional = true,
.required = true,
- .flags = VSH_OFLAG_REQ,
.completer = virshDomainDiskTargetCompleter,
.help = N_("Fully-qualified path of block device")
},
@@ -3064,7 +3055,6 @@ static const vshCmdOptDef opts_domif_setlink[] = {
.type = VSH_OT_STRING,
.positional = true,
.required = true,
- .flags = VSH_OFLAG_REQ,
.completer = virshDomainInterfaceCompleter,
.help = N_("interface device (MAC Address)")
},
@@ -3072,7 +3062,6 @@ static const vshCmdOptDef opts_domif_setlink[] = {
.type = VSH_OT_STRING,
.positional = true,
.required = true,
- .flags = VSH_OFLAG_REQ,
.completer = virshDomainInterfaceStateCompleter,
.help = N_("new
[PATCH 23/23] vsh: Automatically calculate VSH_OFLAG_REQ_OPT
The command line parser automatically requires the option name for any
optional arguments, but for required non-positional ones special
handling needs to be done.
This patch makes the VSH_OFLAG_REQ_OPT flag completely redundant as it
can be calculated.
Signed-off-by: Peter Krempa
---
tools/virsh-backup.c | 1 -
tools/virsh-checkpoint.c | 1 -
tools/virsh-domain-monitor.c | 1 -
tools/virsh-domain.c | 16 +---
tools/virsh-host.c | 2 --
tools/virsh-interface.c | 1 -
tools/virsh-network.c| 2 --
tools/virsh-nodedev.c| 1 -
tools/virsh-nwfilter.c | 2 --
tools/virsh-pool.c | 1 -
tools/virsh-secret.c | 2 --
tools/virsh-snapshot.c | 2 --
tools/virsh-volume.c | 1 -
tools/virt-admin.c | 1 -
tools/vsh.c | 6 +-
tools/vsh.h | 1 -
16 files changed, 6 insertions(+), 35 deletions(-)
diff --git a/tools/virsh-backup.c b/tools/virsh-backup.c
index 7a7834d7ff..f8b11355a1 100644
--- a/tools/virsh-backup.c
+++ b/tools/virsh-backup.c
@@ -107,7 +107,6 @@ static const vshCmdOptDef opts_backup_dumpxml[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
{.name = "xpath",
.type = VSH_OT_STRING,
- .flags = VSH_OFLAG_REQ_OPT,
.completer = virshCompleteEmpty,
.help = N_("xpath expression to filter the XML document")
},
diff --git a/tools/virsh-checkpoint.c b/tools/virsh-checkpoint.c
index 869c071700..ef086fd1a5 100644
--- a/tools/virsh-checkpoint.c
+++ b/tools/virsh-checkpoint.c
@@ -826,7 +826,6 @@ static const vshCmdOptDef opts_checkpoint_dumpxml[] = {
},
{.name = "xpath",
.type = VSH_OT_STRING,
- .flags = VSH_OFLAG_REQ_OPT,
.completer = virshCompleteEmpty,
.help = N_("xpath expression to filter the XML document")
},
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 6c2499fb9f..ff2872d497 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -277,7 +277,6 @@ static const vshCmdOptDef opts_dommemstat[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
{.name = "period",
.type = VSH_OT_INT,
- .flags = VSH_OFLAG_REQ_OPT,
.help = N_("period in seconds to set collection")
},
VIRSH_COMMON_OPT_CONFIG(N_("affect next boot")),
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 7c2613c85b..65c67ff038 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -3436,7 +3436,6 @@ static const vshCmdOptDef opts_dom_pm_suspend[] = {
},
{.name = "duration",
.type = VSH_OT_INT,
- .flags = VSH_OFLAG_REQ_OPT,
.help = N_("duration in seconds")
},
{.name = NULL}
@@ -4471,7 +4470,6 @@ static const vshCmdOptDef opts_save_image_dumpxml[] = {
},
{.name = "xpath",
.type = VSH_OT_STRING,
- .flags = VSH_OFLAG_REQ_OPT,
.completer = virshCompleteEmpty,
.help = N_("xpath expression to filter the XML document")
},
@@ -4877,7 +4875,6 @@ static const vshCmdOptDef opts_managed_save_dumpxml[] = {
},
{.name = "xpath",
.type = VSH_OT_STRING,
- .flags = VSH_OFLAG_REQ_OPT,
.completer = virshCompleteEmpty,
.help = N_("xpath expression to filter the XML document")
},
@@ -4987,12 +4984,10 @@ static const vshCmdOptDef opts_schedinfo[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(0),
{.name = "weight",
.type = VSH_OT_INT,
- .flags = VSH_OFLAG_REQ_OPT,
.help = N_("weight for XEN_CREDIT")
},
{.name = "cap",
.type = VSH_OT_INT,
- .flags = VSH_OFLAG_REQ_OPT,
.help = N_("cap for XEN_CREDIT")
},
VIRSH_COMMON_OPT_CURRENT(N_("get/set current scheduler info")),
@@ -8500,13 +8495,11 @@ static const vshCmdOptDef opts_send_key[] = {
VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
{.name = "codeset",
.type = VSH_OT_STRING,
- .flags = VSH_OFLAG_REQ_OPT,
.completer = virshCodesetNameCompleter,
.help = N_("the codeset of keycodes, default:linux")
},
{.name = "holdtime",
.type = VSH_OT_INT,
- .flags = VSH_OFLAG_REQ_OPT,
.help = N_("the time (in milliseconds) how long the keys will be held")
},
{.name = "keycode",
@@ -9436,12 +9429,10 @@ static const vshCmdOptDef opts_domsetlaunchsecstate[] =
{
VIRSH_COMMON_OPT_DOMAIN_FULL(0),
{.name = "secrethdr",
.type = VSH_OT_STRING,
- .flags = VSH_OFLAG_REQ_OPT,
.help = N_("path to file containing the secret header"),
},
{.name = "secret",
.type = VSH_OT_STRING,
- .flags = VSH_OFLAG_REQ_OPT,
.help = N_("path to file containing the secret"),
},
{.name = "set-address",
@@ -9613,7 +9604,6 @@ static const vshCmdOptDef opts_qemu_monitor_command[] = {
},
{.name = "pass-fds",
.type = VSH_OT_STRING,
- .flags = VSH_OFLAG_REQ_OPT,
.completer = virshComple
Re: [PATCH v2] ch: Enable hyperv hypervisor
On 3/8/24 16:55, Praveen K Paladugu wrote:
>
>
> On 3/8/2024 6:06 AM, Michal Prívozník wrote:
>> On 2/20/24 23:06, Praveen K Paladugu wrote:
>>> From: Praveen K Paladugu
>>>
>>> Cloud-Hypervisor is capable of running VMs with kvm or mshv as the
>>> hypervisor on Linux Host. Guest to hypevisor ABI with mshv hypervisor is
>>> the same as in the case of VIR_DOMAIN_VIRT_HYPERV. So,
>>> VIR_DOMAIN_VIRT_HYPERV
>>> type will be reused to represent the config with Linux Host and mshv
>>> as the
>>> hypervisor.
>>>
>>> While initializing ch driver, check if either of /dev/kvm or /dev/mshv
>>> device is present on the host. Before starting ch domains, check if the
>>> requested hypervisor device is present on the host.
>>>
>>> Users can specify hypervisor in ch guests's domain definitions like
>>> below:
>>>
>>>
>>>
>>> _or_
>>>
>>>
>>>
>>> Signed-off-by: Praveen K Paladugu
>>> Signed-off-by: Praveen K Paladugu
>>> ---
>>> src/ch/ch_conf.c | 2 ++
>>> src/ch/ch_driver.c | 7 +++
>>> src/ch/ch_process.c | 35 +++
>>> 3 files changed, 44 insertions(+)
>>>
>>> diff --git a/src/ch/ch_conf.c b/src/ch/ch_conf.c
>>> index f421af5121..1911ae8f8b 100644
>>> --- a/src/ch/ch_conf.c
>>> +++ b/src/ch/ch_conf.c
>>> @@ -69,6 +69,8 @@ virCaps *virCHDriverCapsInit(void)
>>> virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_KVM,
>>> NULL, NULL, 0, NULL);
>>> + virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_HYPERV,
>>> + NULL, NULL, 0, NULL);
>>
>> 1: This sets support for both virtTypes unconditionally even though only
>> one might be supported. Problem with this approach is: I, as an user,
>> check for supported virtTypes (e.g. via 'virsh capabilities') find
>> hyperv supported only to get an error when trying to start such domain.
>>
>>> return g_steal_pointer(&caps);
>>> }
>>> diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
>>> index 96de5044ac..d6294c76ee 100644
>>> --- a/src/ch/ch_driver.c
>>> +++ b/src/ch/ch_driver.c
>>> @@ -32,6 +32,7 @@
>>> #include "viraccessapicheck.h"
>>> #include "virchrdev.h"
>>> #include "virerror.h"
>>> +#include "virfile.h"
>>> #include "virlog.h"
>>> #include "virobject.h"
>>> #include "virtypedparam.h"
>>> @@ -876,6 +877,12 @@ static int chStateInitialize(bool privileged,
>>> return -1;
>>> }
>>> + if (!(virFileExists("/dev/kvm") || virFileExists("/dev/mshv"))) {
>>> + virReportError(VIR_ERR_DEVICE_MISSING, "%s",
>>
>> VIR_ERR_DEVICE_MISSING code should be used for cases where a device is
>> looked up in domain config but it's not found (e.g. on hotunplug).
>> Though it's used in one other (unrelated?) case too:
>> virMediatedDeviceNew() - which is transitively called from domain
>> handling code.
>>
>> But more importantly, this check needs to go to caps init [1] and here
>> we should merely check whether caps has at least one of the virtTypes
>> set (e.g. via virCapabilitiesDomainSupported()).
>>
>>
>>> + _("/dev/kvm and /dev/mshv. ch driver failed
>>> to initialize."));
>>> + return VIR_DRV_STATE_INIT_ERROR;
>>> + }
>>> +
>>> ch_driver = g_new0(virCHDriver, 1);
>>> if (virMutexInit(&ch_driver->lock) < 0) {
>>> diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c
>>> index 3bde9d9dcf..640f72a9ca 100644
>>> --- a/src/ch/ch_process.c
>>> +++ b/src/ch/ch_process.c
>>> @@ -637,6 +637,37 @@ chProcessAddNetworkDevices(virCHDriver *driver,
>>> return 0;
>>> }
>>> +/**
>>> + * virCHProcessStartValidate:
>>> + * @vm: domain object
>>> + *
>>> + * Checks done before starting a VM.
>>> + *
>>> + * Returns 0 on success or -1 in case of error
>>> + */
>>> +static int virCHProcessStartValidate(virDomainObj *vm)
>>> +{
>>> + if (vm->def->virtType == VIR_DOMAIN_VIRT_KVM) {
>>> + VIR_DEBUG("Checking for KVM availability");
>>> + if (!virFileExists("/dev/kvm")) {
>>
>> We should check capabilities instead, just like I'm suggesting above.
>
> As 'mshv' and 'kvm' can be configured to be loadable modules, don't you
> think there is value in checking if these devices still exist before
> starting a CH process?
Good point. I thought capabilities are refreshed when a domain is to be
created, but that's not the case for CH driver.
And comparing the QEMU and CH code:
1) the fact whether system is capable of KVM is stored in virQEMUCaps
and only when virCaps are constructed/refreshed this information is
mirrored into the latter,
2) virQEMUCaps is stored in a cache (virFileCache) and whenever a
reference to virQEMUCaps is requested, the cache is validated, i.e.
basic checks are run (virQEMUCapsIsValid()) , like has mtime of the qemu
binary changed? is the kvm module still loaded and so on.
3) We don't have such checks in CH driver, so if the virchd is started
and then the cloud-hypervisor binary is upgraded the daemon doesn't
reconstruct (version based
[libvirt PATCH 1/3] qemu: virtiofs: do not crash if cgroups are missing
On domain startup, qemuSetupCgroupForExtDevices checks if a cgroup controller is present and skips the setup if not. Add a similar check to qemuVirtioFSSetupCgroup to prevent crashing when hotplugging a virtiofs filesystem. Signed-off-by: Ján Tomko --- src/qemu/qemu_virtiofs.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c index d539d0a192..15dea3bb57 100644 --- a/src/qemu/qemu_virtiofs.c +++ b/src/qemu/qemu_virtiofs.c @@ -353,6 +353,9 @@ qemuVirtioFSSetupCgroup(virDomainObj *vm, pid_t pid = -1; int rc; +if (!cgroup) +return 0; + if (!(pidfile = qemuVirtioFSCreatePidFilename(vm, fs->info.alias))) return -1; -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[libvirt PATCH 3/3] qemu: virtiofs: error out if getting the group or user name fails
Signed-off-by: Ján Tomko --- src/qemu/qemu_virtiofs.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c index d80cddd3ba..78897d8177 100644 --- a/src/qemu/qemu_virtiofs.c +++ b/src/qemu/qemu_virtiofs.c @@ -388,6 +388,9 @@ qemuVirtioFSPrepareIdMap(virDomainFSDef *fs) username = virGetUserName(euid); groupname = virGetGroupName(egid); +if (!username || !groupname) +return -1; + fs->idmap.uidmap = g_new0(virDomainIdMapEntry, 2); fs->idmap.gidmap = g_new0(virDomainIdMapEntry, 2); -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[libvirt PATCH 2/3] qemu: virtiofs: set correct label when creating the socket
Use svirt_t instead of virtd_t, since virtd_t is not available in the
session mode and qemu with svirt_t won't be able to talk to unconfined_t
socket.
Signed-off-by: Ján Tomko
---
src/qemu/qemu_virtiofs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c
index 15dea3bb57..d80cddd3ba 100644
--- a/src/qemu/qemu_virtiofs.c
+++ b/src/qemu/qemu_virtiofs.c
@@ -102,7 +102,7 @@ qemuVirtioFSOpenChardev(virQEMUDriver *driver,
chrdev->data.nix.listen = true;
chrdev->data.nix.path = g_strdup(socket_path);
-if (qemuSecuritySetDaemonSocketLabel(driver->securityManager, vm->def) < 0)
+if (qemuSecuritySetSocketLabel(driver->securityManager, vm->def) < 0)
goto cleanup;
fd = qemuOpenChrChardevUNIXSocket(chrdev);
if (fd < 0) {
--
2.44.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[libvirt PATCH 0/3] qemu: virtiofs: fixes for session mode
Ján Tomko (3): qemu: virtiofs: do not crash if cgroups are missing qemu: virtiofs: set correct label when creating the socket qemu: virtiofs: error out if getting the group or user name fails src/qemu/qemu_virtiofs.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] Remove VIR_FREE in favor of g_autofree in some functions in libvrit-domain.c
Hello Daniel, Peter,
Apologies for the accidental noreply mail in the CC, it's probably the
"supresscc = self" line that I copied blindly into my send-email git config.
Also, many apologies for forgetting to at least compile the change. I have read
the guidelines before but I forgot.
Question: After I amend the commit with the right changes, should I simply call
"git publish" again?
I tested this in a small repo I made and the behaviour is that "git publish"
starts a new email chain with [PATCH v2]. Is this what should happen or is it
required for the new PATCH to be a reply to this chain?
Cheers,
Mostafa
From: Daniel P. Berrangé
Sent: Monday, March 11, 2024 9:24 AM
To: Mostafa
Cc: devel@lists.libvirt.org
Subject: Re: [PATCH] Remove VIR_FREE in favor of g_autofree in some functions
in libvrit-domain.c
On Mon, Mar 11, 2024 at 02:15:32AM +0200, Mostafa wrote:
> From: مصطفي محمود كمال الدين <48567303+most...@users.noreply.github.com>
>
> ---
> src/libvirt-domain.c | 32
> 1 file changed, 8 insertions(+), 24 deletions(-)
>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 83abad251e..9b68a7ac95 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -884,7 +884,7 @@ virDomainSave(virDomainPtr domain, const char *to)
>
> if (conn->driver->domainSave) {
> int ret;
> -char *absolute_to;
> +g_autofree char *absolute_to;
All variables declared with 'g_autofree' *must* be initialized
at time of declaration, so you need to add ' = NULL' here and
to all the other similar changes.
>
> /* We must absolutize the file path as the save is done out of
> process */
> if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
> @@ -895,8 +895,6 @@ virDomainSave(virDomainPtr domain, const char *to)
>
> ret = conn->driver->domainSave(domain, absolute_to);
>
> -VIR_FREE(absolute_to);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -974,7 +972,7 @@ virDomainSaveFlags(virDomainPtr domain, const char *to,
>
> if (conn->driver->domainSaveFlags) {
> int ret;
> -char *absolute_to;
> +g_autofree char *absolute_to;
>
> /* We must absolutize the file path as the save is done out of
> process */
> if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
> @@ -985,8 +983,6 @@ virDomainSaveFlags(virDomainPtr domain, const char *to,
>
> ret = conn->driver->domainSaveFlags(domain, absolute_to, dxml,
> flags);
>
> -VIR_FREE(absolute_to);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -1076,7 +1072,7 @@ virDomainRestore(virConnectPtr conn, const char *from)
>
> if (conn->driver->domainRestore) {
> int ret;
> -char *absolute_from;
> +g_autofree char *absolute_from;
>
> /* We must absolutize the file path as the restore is done out of
> process */
> if (!(absolute_from = g_canonicalize_filename(from, NULL))) {
> @@ -1087,8 +1083,6 @@ virDomainRestore(virConnectPtr conn, const char *from)
>
> ret = conn->driver->domainRestore(conn, absolute_from);
>
> -VIR_FREE(absolute_from);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -1156,7 +1150,7 @@ virDomainRestoreFlags(virConnectPtr conn, const char
> *from, const char *dxml,
>
> if (conn->driver->domainRestoreFlags) {
> int ret;
> -char *absolute_from;
> +g_autofree char *absolute_from;
>
> /* We must absolutize the file path as the restore is done out of
> process */
> if (!(absolute_from = g_canonicalize_filename(from, NULL))) {
> @@ -1168,8 +1162,6 @@ virDomainRestoreFlags(virConnectPtr conn, const char
> *from, const char *dxml,
> ret = conn->driver->domainRestoreFlags(conn, absolute_from, dxml,
> flags);
>
> -VIR_FREE(absolute_from);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -1263,7 +1255,7 @@ virDomainSaveImageGetXMLDesc(virConnectPtr conn, const
> char *file,
>
> if (conn->driver->domainSaveImageGetXMLDesc) {
> char *ret;
> -char *absolute_file;
> +g_autofree char *absolute_file;
>
> /* We must absolutize the file path as the read is done out of
> process */
> if (!(absolute_file = g_canonicalize_filename(file, NULL))) {
> @@ -1275,8 +1267,6 @@ virDomainSaveImageGetXMLDesc(virConnectPtr conn, const
> char *file,
> ret = conn->driver->domainSaveImageGetXMLDesc(conn, absolute_file,
>flags);
>
> -VIR_FREE(absolute_file);
> -
> if (!ret)
> goto error;
> return ret;
> @@ -1338,7 +1328,7 @@ virDomainSaveImageDefineXML(virConnectPtr conn, const
> char *file,
>
> if (conn->drive
Re: [PATCH] Remove VIR_FREE in favor of g_autofree in some functions in libvrit-domain.c
After seeing a couple of patches in the list, it looks like [PATCH v2] is the
usual expected behavior. So, I will proceed with that.
Many thanks. Sorry for any inconvenience.
From: m kamal
Sent: Monday, March 11, 2024 3:45 PM
To: Daniel P. Berrangé
Cc: devel@lists.libvirt.org ; pkre...@redhat.com
Subject: Re: [PATCH] Remove VIR_FREE in favor of g_autofree in some functions
in libvrit-domain.c
Hello Daniel, Peter,
Apologies for the accidental noreply mail in the CC, it's probably the
"supresscc = self" line that I copied blindly into my send-email git config.
Also, many apologies for forgetting to at least compile the change. I have read
the guidelines before but I forgot.
Question: After I amend the commit with the right changes, should I simply call
"git publish" again?
I tested this in a small repo I made and the behaviour is that "git publish"
starts a new email chain with [PATCH v2]. Is this what should happen or is it
required for the new PATCH to be a reply to this chain?
Cheers,
Mostafa
From: Daniel P. Berrangé
Sent: Monday, March 11, 2024 9:24 AM
To: Mostafa
Cc: devel@lists.libvirt.org
Subject: Re: [PATCH] Remove VIR_FREE in favor of g_autofree in some functions
in libvrit-domain.c
On Mon, Mar 11, 2024 at 02:15:32AM +0200, Mostafa wrote:
> From: مصطفي محمود كمال الدين <48567303+most...@users.noreply.github.com>
>
> ---
> src/libvirt-domain.c | 32
> 1 file changed, 8 insertions(+), 24 deletions(-)
>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 83abad251e..9b68a7ac95 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -884,7 +884,7 @@ virDomainSave(virDomainPtr domain, const char *to)
>
> if (conn->driver->domainSave) {
> int ret;
> -char *absolute_to;
> +g_autofree char *absolute_to;
All variables declared with 'g_autofree' *must* be initialized
at time of declaration, so you need to add ' = NULL' here and
to all the other similar changes.
>
> /* We must absolutize the file path as the save is done out of
> process */
> if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
> @@ -895,8 +895,6 @@ virDomainSave(virDomainPtr domain, const char *to)
>
> ret = conn->driver->domainSave(domain, absolute_to);
>
> -VIR_FREE(absolute_to);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -974,7 +972,7 @@ virDomainSaveFlags(virDomainPtr domain, const char *to,
>
> if (conn->driver->domainSaveFlags) {
> int ret;
> -char *absolute_to;
> +g_autofree char *absolute_to;
>
> /* We must absolutize the file path as the save is done out of
> process */
> if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
> @@ -985,8 +983,6 @@ virDomainSaveFlags(virDomainPtr domain, const char *to,
>
> ret = conn->driver->domainSaveFlags(domain, absolute_to, dxml,
> flags);
>
> -VIR_FREE(absolute_to);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -1076,7 +1072,7 @@ virDomainRestore(virConnectPtr conn, const char *from)
>
> if (conn->driver->domainRestore) {
> int ret;
> -char *absolute_from;
> +g_autofree char *absolute_from;
>
> /* We must absolutize the file path as the restore is done out of
> process */
> if (!(absolute_from = g_canonicalize_filename(from, NULL))) {
> @@ -1087,8 +1083,6 @@ virDomainRestore(virConnectPtr conn, const char *from)
>
> ret = conn->driver->domainRestore(conn, absolute_from);
>
> -VIR_FREE(absolute_from);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -1156,7 +1150,7 @@ virDomainRestoreFlags(virConnectPtr conn, const char
> *from, const char *dxml,
>
> if (conn->driver->domainRestoreFlags) {
> int ret;
> -char *absolute_from;
> +g_autofree char *absolute_from;
>
> /* We must absolutize the file path as the restore is done out of
> process */
> if (!(absolute_from = g_canonicalize_filename(from, NULL))) {
> @@ -1168,8 +1162,6 @@ virDomainRestoreFlags(virConnectPtr conn, const char
> *from, const char *dxml,
> ret = conn->driver->domainRestoreFlags(conn, absolute_from, dxml,
> flags);
>
> -VIR_FREE(absolute_from);
> -
> if (ret < 0)
> goto error;
> return ret;
> @@ -1263,7 +1255,7 @@ virDomainSaveImageGetXMLDesc(virConnectPtr conn, const
> char *file,
>
> if (conn->driver->domainSaveImageGetXMLDesc) {
> char *ret;
> -char *absolute_file;
> +g_autofree char *absolute_file;
>
> /* We must absolutize the file path as the read is done out of
> process */
> if (!(absolute_file = g_canonicalize_filename(file, NULL))) {
> @@ -1275,8 +1267,6 @@ virDom
[PATCH-for-9.0] docs: Deprecate the pseries-2.12 machines
pSeries machines before 3.0 have complex migration back
compatibility code we'd like to get ride of. The last
one is 2.12, which is 6 years old. We just deprecated up
to the 2.11 machine in commit 1392617d35 ("spapr: Tag
pseries-2.1 - 2.11 machines as deprecated").
Take to opportunity to also deprecate the 2.12 machines.
Signed-off-by: Philippe Mathieu-Daudé
---
In 2025 I'd like to get ride of the code related to:
include/hw/ppc/spapr_cpu_core.h:31:bool pre_3_0_migration; /* older
machine don't know about SpaprCpuState */
---
docs/about/deprecated.rst | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index dfd681cd02..65111513cc 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -237,13 +237,13 @@ The Nios II architecture is orphan.
The machine is no longer in existence and has been long unmaintained
in QEMU. This also holds for the TC51828 16MiB flash that it uses.
-``pseries-2.1`` up to ``pseries-2.11`` (since 9.0)
+``pseries-2.1`` up to ``pseries-2.12`` (since 9.0)
''
-Older pseries machines before version 2.12 have undergone many changes
+Older pseries machines before version 3.0 have undergone many changes
to correct issues, mostly regarding migration compatibility. These are
no longer maintained and removing them will make the code easier to
-read and maintain. Use versions 2.12 and above as a replacement.
+read and maintain. Use versions 3.0 and above as a replacement.
Backend options
---
--
2.41.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v2] Remove VIR_FREE in favor of g_autofree in some functions in libvrit-domain.c
Signed-off-by: Mostafa
---
src/libvirt-domain.c | 32
1 file changed, 8 insertions(+), 24 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 83abad251e..4ba3563c9e 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -884,7 +884,7 @@ virDomainSave(virDomainPtr domain, const char *to)
if (conn->driver->domainSave) {
int ret;
-char *absolute_to;
+g_autofree char *absolute_to = NULL;
/* We must absolutize the file path as the save is done out of process
*/
if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
@@ -895,8 +895,6 @@ virDomainSave(virDomainPtr domain, const char *to)
ret = conn->driver->domainSave(domain, absolute_to);
-VIR_FREE(absolute_to);
-
if (ret < 0)
goto error;
return ret;
@@ -974,7 +972,7 @@ virDomainSaveFlags(virDomainPtr domain, const char *to,
if (conn->driver->domainSaveFlags) {
int ret;
-char *absolute_to;
+g_autofree char *absolute_to = NULL;
/* We must absolutize the file path as the save is done out of process
*/
if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
@@ -985,8 +983,6 @@ virDomainSaveFlags(virDomainPtr domain, const char *to,
ret = conn->driver->domainSaveFlags(domain, absolute_to, dxml, flags);
-VIR_FREE(absolute_to);
-
if (ret < 0)
goto error;
return ret;
@@ -1076,7 +1072,7 @@ virDomainRestore(virConnectPtr conn, const char *from)
if (conn->driver->domainRestore) {
int ret;
-char *absolute_from;
+g_autofree char *absolute_from = NULL;
/* We must absolutize the file path as the restore is done out of
process */
if (!(absolute_from = g_canonicalize_filename(from, NULL))) {
@@ -1087,8 +1083,6 @@ virDomainRestore(virConnectPtr conn, const char *from)
ret = conn->driver->domainRestore(conn, absolute_from);
-VIR_FREE(absolute_from);
-
if (ret < 0)
goto error;
return ret;
@@ -1156,7 +1150,7 @@ virDomainRestoreFlags(virConnectPtr conn, const char
*from, const char *dxml,
if (conn->driver->domainRestoreFlags) {
int ret;
-char *absolute_from;
+g_autofree char *absolute_from = NULL;
/* We must absolutize the file path as the restore is done out of
process */
if (!(absolute_from = g_canonicalize_filename(from, NULL))) {
@@ -1168,8 +1162,6 @@ virDomainRestoreFlags(virConnectPtr conn, const char
*from, const char *dxml,
ret = conn->driver->domainRestoreFlags(conn, absolute_from, dxml,
flags);
-VIR_FREE(absolute_from);
-
if (ret < 0)
goto error;
return ret;
@@ -1263,7 +1255,7 @@ virDomainSaveImageGetXMLDesc(virConnectPtr conn, const
char *file,
if (conn->driver->domainSaveImageGetXMLDesc) {
char *ret;
-char *absolute_file;
+g_autofree char *absolute_file = NULL;
/* We must absolutize the file path as the read is done out of process
*/
if (!(absolute_file = g_canonicalize_filename(file, NULL))) {
@@ -1275,8 +1267,6 @@ virDomainSaveImageGetXMLDesc(virConnectPtr conn, const
char *file,
ret = conn->driver->domainSaveImageGetXMLDesc(conn, absolute_file,
flags);
-VIR_FREE(absolute_file);
-
if (!ret)
goto error;
return ret;
@@ -1338,7 +1328,7 @@ virDomainSaveImageDefineXML(virConnectPtr conn, const
char *file,
if (conn->driver->domainSaveImageDefineXML) {
int ret;
-char *absolute_file;
+g_autofree char *absolute_file = NULL;
/* We must absolutize the file path as the read is done out of process
*/
if (!(absolute_file = g_canonicalize_filename(file, NULL))) {
@@ -1350,8 +1340,6 @@ virDomainSaveImageDefineXML(virConnectPtr conn, const
char *file,
ret = conn->driver->domainSaveImageDefineXML(conn, absolute_file,
dxml, flags);
-VIR_FREE(absolute_file);
-
if (ret < 0)
goto error;
return ret;
@@ -1415,7 +1403,7 @@ virDomainCoreDump(virDomainPtr domain, const char *to,
unsigned int flags)
if (conn->driver->domainCoreDump) {
int ret;
-char *absolute_to;
+g_autofree char *absolute_to = NULL;
/* We must absolutize the file path as the save is done out of process
*/
if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
@@ -1426,8 +1414,6 @@ virDomainCoreDump(virDomainPtr domain, const char *to,
unsigned int flags)
ret = conn->driver->domainCoreDump(domain, absolute_to, flags);
-VIR_FREE(absolute_to);
-
if (ret < 0)
goto error;
return ret
