Re: [PATCH 09/10] virsh: Introduce update-memory command
On 2/2/21 2:41 PM, Peter Krempa wrote:
On Fri, Jan 22, 2021 at 13:50:31 +0100, Michal Privoznik wrote:
New 'update-memory' command is introduced which aims on making it
user friendly to change device. So far I just need to
change so I'm introducing --requested-size only; but
the idea is that this is extensible for other cases too. For
instance, want to change ? Nnew --my-element
argument can be easily introduced.
Signed-off-by: Michal Privoznik
---
docs/manpages/virsh.rst | 31
tools/virsh-domain.c| 154
2 files changed, 185 insertions(+)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index e3afa48f7b..32639e34ff 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -4891,6 +4891,37 @@ results as some fields may be autogenerated and thus
match devices other than
expected.
+update-memory
update-memory-device-perhaps?
Okay.
+-
+
+**Syntax:**
+
+::
+
+ update-memory domain [--print-xml] [--alias alias]
+ [[--live] [--config] | [--current]]
+ [--requested-size size]
+
+Update values for a device. Not to be confused with overall
+domain memory which is tuned via ``setmem`` and ``setmaxmem``.
So that you don't have to add this disclaimer?
+This command finds device inside given *domain*, changes
+requested values and passes updated device XML to daemon. If *--print-xml* is
+specified then the device is not changed, but the updated device XML is printed
+to stdout. If there are more than one devices in *domain* use
+*--alias* to select the desired one.
+
+If *--live* is specified, affect a running domain.
+If *--config* is specified, affect the next startup of a persistent guest.
+If *--current* is specified, it is equivalent to either *--live* or
+*--config*, depending on the current state of the guest.
+Both *--live* and *--config* flags may be given, but *--current* is
+exclusive. Not specifying any flag is the same as specifying *--current*.
+
+If *--requested-size* is specified then under memory target is
+changed to requested *size* (as scaled integer, see ``NOTES`` above). It
+defaults to kibibytes if no suffix is provided.
... this document doesn't mention that it works only for the property of
virtio-mem. Users of virsh tend to not read other docs, so please add
it.
I can do that, but virsh errors out if it didn't find any ,
like this:
virsh # update-memory gentoo --alias virtiopmem0 --print-xml
--requested-size 5
error: virtio-mem device is missing
Okay, the error message is misleading a bit (will fix it), because I was
trying to modify virtio-pmem.
How badly do we want to protect users from themselves?
+
+
change-media
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 9746117bdb..0b32e6f408 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9128,6 +9128,154 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd)
return ret;
}
+
+/*
+ * "update-memory" command
+ */
+static const vshCmdInfo info_update_memory[] = {
+{.name = "help",
+ .data = N_("update memory device of a domain")
+},
+{.name = "desc",
+ .data = N_("Update values of a memory device of a domain")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_update_memory[] = {
+VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+VIRSH_COMMON_OPT_DOMAIN_CONFIG,
+VIRSH_COMMON_OPT_DOMAIN_LIVE,
+VIRSH_COMMON_OPT_DOMAIN_CURRENT,
+{.name = "print-xml",
+ .type = VSH_OT_BOOL,
+ .help = N_("print updated memory device XML instead of executing the
change")
+},
+{.name = "alias",
+ .type = VSH_OT_STRING,
+ .completer = virshDomainDeviceAliasCompleter,
This completes also non-memory devices.
Yes, and I am okay with that. Alias is optional and needed only if two
or more devices exist inside domain (regardless of their
model). We already allow completion of mutually exclusive --options
(because the exclusivity is not expressed in --options definition). We
could have .completer_flags as an ORed set of virDomainDeviceType,
except not really because that's an enum with continuous values and not
a bitmask. I could write a new completer, but if we want to do that for
every device (eventually) we would need ~20 different completers. Waste
of time.
Again, I don't think we should guard users from shooting themselves into
foot.
+ .help = N_("memory device alias"),
+},
+{.name = "requested-size",
+ .type = VSH_OT_INT,
+ .help = N_("new value of size, as scaled integer (default
KiB)")
+},
+{.name = NULL}
+};
+
+static int
+virshGetUpdatedMemoryXML(char **updatedMemoryXML,
+ vshControl *ctl,
+ const vshCmd *cmd,
+ virDomainPtr dom,
+ unsigned int flags)
+{
+const char *alias = NULL;
+g_autoptr(xmlDoc) doc = NULL;
+
Re: [PATCH 09/10] virsh: Introduce update-memory command
On Fri, Jan 22, 2021 at 13:50:31 +0100, Michal Privoznik wrote:
> New 'update-memory' command is introduced which aims on making it
> user friendly to change device. So far I just need to
> change so I'm introducing --requested-size only; but
> the idea is that this is extensible for other cases too. For
> instance, want to change ? Nnew --my-element
> argument can be easily introduced.
>
> Signed-off-by: Michal Privoznik
> ---
> docs/manpages/virsh.rst | 31
> tools/virsh-domain.c| 154
> 2 files changed, 185 insertions(+)
>
> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> index e3afa48f7b..32639e34ff 100644
> --- a/docs/manpages/virsh.rst
> +++ b/docs/manpages/virsh.rst
> @@ -4891,6 +4891,37 @@ results as some fields may be autogenerated and thus
> match devices other than
> expected.
>
>
> +update-memory
update-memory-device-perhaps?
> +-
> +
> +**Syntax:**
> +
> +::
> +
> + update-memory domain [--print-xml] [--alias alias]
> + [[--live] [--config] | [--current]]
> + [--requested-size size]
> +
> +Update values for a device. Not to be confused with overall
> +domain memory which is tuned via ``setmem`` and ``setmaxmem``.
So that you don't have to add this disclaimer?
> +This command finds device inside given *domain*, changes
> +requested values and passes updated device XML to daemon. If *--print-xml* is
> +specified then the device is not changed, but the updated device XML is
> printed
> +to stdout. If there are more than one devices in *domain* use
> +*--alias* to select the desired one.
> +
> +If *--live* is specified, affect a running domain.
> +If *--config* is specified, affect the next startup of a persistent guest.
> +If *--current* is specified, it is equivalent to either *--live* or
> +*--config*, depending on the current state of the guest.
> +Both *--live* and *--config* flags may be given, but *--current* is
> +exclusive. Not specifying any flag is the same as specifying *--current*.
> +
> +If *--requested-size* is specified then under memory target
> is
> +changed to requested *size* (as scaled integer, see ``NOTES`` above). It
> +defaults to kibibytes if no suffix is provided.
... this document doesn't mention that it works only for the property of
virtio-mem. Users of virsh tend to not read other docs, so please add
it.
> +
> +
> change-media
>
>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 9746117bdb..0b32e6f408 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -9128,6 +9128,154 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd)
> return ret;
> }
>
> +
> +/*
> + * "update-memory" command
> + */
> +static const vshCmdInfo info_update_memory[] = {
> +{.name = "help",
> + .data = N_("update memory device of a domain")
> +},
> +{.name = "desc",
> + .data = N_("Update values of a memory device of a domain")
> +},
> +{.name = NULL}
> +};
> +
> +static const vshCmdOptDef opts_update_memory[] = {
> +VIRSH_COMMON_OPT_DOMAIN_FULL(0),
> +VIRSH_COMMON_OPT_DOMAIN_CONFIG,
> +VIRSH_COMMON_OPT_DOMAIN_LIVE,
> +VIRSH_COMMON_OPT_DOMAIN_CURRENT,
> +{.name = "print-xml",
> + .type = VSH_OT_BOOL,
> + .help = N_("print updated memory device XML instead of executing the
> change")
> +},
> +{.name = "alias",
> + .type = VSH_OT_STRING,
> + .completer = virshDomainDeviceAliasCompleter,
This completes also non-memory devices.
> + .help = N_("memory device alias"),
> +},
> +{.name = "requested-size",
> + .type = VSH_OT_INT,
> + .help = N_("new value of size, as scaled integer (default
> KiB)")
> +},
> +{.name = NULL}
> +};
> +
> +static int
> +virshGetUpdatedMemoryXML(char **updatedMemoryXML,
> + vshControl *ctl,
> + const vshCmd *cmd,
> + virDomainPtr dom,
> + unsigned int flags)
> +{
> +const char *alias = NULL;
> +g_autoptr(xmlDoc) doc = NULL;
> +g_autoptr(xmlXPathContext) ctxt = NULL;
> +g_autofree char *xpath = NULL;
> +int nmems;
> +g_autofree xmlNodePtr *mems = NULL;
> +g_autoptr(xmlBuffer) xmlbuf = NULL;
> +unsigned int domainXMLFlags = 0;
> +
> +if (flags & VIR_DOMAIN_AFFECT_CONFIG)
> +domainXMLFlags |= VIR_DOMAIN_XML_INACTIVE;
> +
> +if (virshDomainGetXMLFromDom(ctl, dom, domainXMLFlags, , ) < 0)
> +return -1;
> +
> +if (vshCommandOptStringReq(ctl, cmd, "alias", ) < 0)
> +return -1;
> +
> +if (alias) {
> +xpath =
> g_strdup_printf("/domain/devices/memory[./alias/@name='%s']", alias);
> +} else {
> +xpath = g_strdup("/domain/devices/memory");
> +}
> +
> +nmems = virXPathNodeSet(xpath, ctxt, );
> +if (nmems < 0) {
> +vshSaveLibvirtError();
> +return -1;
> +} else if (nmems == 0) {
>
Re: [PATCH 09/10] virsh: Introduce update-memory command
On 1/22/21 9:50 AM, Michal Privoznik wrote:
New 'update-memory' command is introduced which aims on making it
user friendly to change device. So far I just need to
change so I'm introducing --requested-size only; but
the idea is that this is extensible for other cases too. For
instance, want to change ? Nnew --my-element
argument can be easily introduced.
I think you meant:
"... want to change ? A new --my-element argument ..."
Reviewed-by: Daniel Henrique Barboza
Signed-off-by: Michal Privoznik
---
docs/manpages/virsh.rst | 31
tools/virsh-domain.c| 154
2 files changed, 185 insertions(+)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index e3afa48f7b..32639e34ff 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -4891,6 +4891,37 @@ results as some fields may be autogenerated and thus
match devices other than
expected.
+update-memory
+-
+
+**Syntax:**
+
+::
+
+ update-memory domain [--print-xml] [--alias alias]
+ [[--live] [--config] | [--current]]
+ [--requested-size size]
+
+Update values for a device. Not to be confused with overall
+domain memory which is tuned via ``setmem`` and ``setmaxmem``.
+This command finds device inside given *domain*, changes
+requested values and passes updated device XML to daemon. If *--print-xml* is
+specified then the device is not changed, but the updated device XML is printed
+to stdout. If there are more than one devices in *domain* use
+*--alias* to select the desired one.
+
+If *--live* is specified, affect a running domain.
+If *--config* is specified, affect the next startup of a persistent guest.
+If *--current* is specified, it is equivalent to either *--live* or
+*--config*, depending on the current state of the guest.
+Both *--live* and *--config* flags may be given, but *--current* is
+exclusive. Not specifying any flag is the same as specifying *--current*.
+
+If *--requested-size* is specified then under memory target is
+changed to requested *size* (as scaled integer, see ``NOTES`` above). It
+defaults to kibibytes if no suffix is provided.
+
+
change-media
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 9746117bdb..0b32e6f408 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9128,6 +9128,154 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd)
return ret;
}
+
+/*
+ * "update-memory" command
+ */
+static const vshCmdInfo info_update_memory[] = {
+{.name = "help",
+ .data = N_("update memory device of a domain")
+},
+{.name = "desc",
+ .data = N_("Update values of a memory device of a domain")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_update_memory[] = {
+VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+VIRSH_COMMON_OPT_DOMAIN_CONFIG,
+VIRSH_COMMON_OPT_DOMAIN_LIVE,
+VIRSH_COMMON_OPT_DOMAIN_CURRENT,
+{.name = "print-xml",
+ .type = VSH_OT_BOOL,
+ .help = N_("print updated memory device XML instead of executing the
change")
+},
+{.name = "alias",
+ .type = VSH_OT_STRING,
+ .completer = virshDomainDeviceAliasCompleter,
+ .help = N_("memory device alias"),
+},
+{.name = "requested-size",
+ .type = VSH_OT_INT,
+ .help = N_("new value of size, as scaled integer (default
KiB)")
+},
+{.name = NULL}
+};
+
+static int
+virshGetUpdatedMemoryXML(char **updatedMemoryXML,
+ vshControl *ctl,
+ const vshCmd *cmd,
+ virDomainPtr dom,
+ unsigned int flags)
+{
+const char *alias = NULL;
+g_autoptr(xmlDoc) doc = NULL;
+g_autoptr(xmlXPathContext) ctxt = NULL;
+g_autofree char *xpath = NULL;
+int nmems;
+g_autofree xmlNodePtr *mems = NULL;
+g_autoptr(xmlBuffer) xmlbuf = NULL;
+unsigned int domainXMLFlags = 0;
+
+if (flags & VIR_DOMAIN_AFFECT_CONFIG)
+domainXMLFlags |= VIR_DOMAIN_XML_INACTIVE;
+
+if (virshDomainGetXMLFromDom(ctl, dom, domainXMLFlags, , ) < 0)
+return -1;
+
+if (vshCommandOptStringReq(ctl, cmd, "alias", ) < 0)
+return -1;
+
+if (alias) {
+xpath = g_strdup_printf("/domain/devices/memory[./alias/@name='%s']",
alias);
+} else {
+xpath = g_strdup("/domain/devices/memory");
+}
+
+nmems = virXPathNodeSet(xpath, ctxt, );
+if (nmems < 0) {
+vshSaveLibvirtError();
+return -1;
+} else if (nmems == 0) {
+vshError(ctl, _("no memory device found"));
+return -1;
+} else if (nmems > 1) {
+vshError(ctl, _("multiple memory devices found, use --alias to select
one"));
+return -1;
+}
+
+ctxt->node = mems[0];
+
+if (vshCommandOptBool(cmd, "requested-size")) {
+xmlNodePtr requestedSizeNode;
+g_autofree char *kibibytesStr = NULL;
+unsigned long long bytes = 0;
+
[PATCH 09/10] virsh: Introduce update-memory command
New 'update-memory' command is introduced which aims on making it
user friendly to change device. So far I just need to
change so I'm introducing --requested-size only; but
the idea is that this is extensible for other cases too. For
instance, want to change ? Nnew --my-element
argument can be easily introduced.
Signed-off-by: Michal Privoznik
---
docs/manpages/virsh.rst | 31
tools/virsh-domain.c| 154
2 files changed, 185 insertions(+)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index e3afa48f7b..32639e34ff 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -4891,6 +4891,37 @@ results as some fields may be autogenerated and thus
match devices other than
expected.
+update-memory
+-
+
+**Syntax:**
+
+::
+
+ update-memory domain [--print-xml] [--alias alias]
+ [[--live] [--config] | [--current]]
+ [--requested-size size]
+
+Update values for a device. Not to be confused with overall
+domain memory which is tuned via ``setmem`` and ``setmaxmem``.
+This command finds device inside given *domain*, changes
+requested values and passes updated device XML to daemon. If *--print-xml* is
+specified then the device is not changed, but the updated device XML is printed
+to stdout. If there are more than one devices in *domain* use
+*--alias* to select the desired one.
+
+If *--live* is specified, affect a running domain.
+If *--config* is specified, affect the next startup of a persistent guest.
+If *--current* is specified, it is equivalent to either *--live* or
+*--config*, depending on the current state of the guest.
+Both *--live* and *--config* flags may be given, but *--current* is
+exclusive. Not specifying any flag is the same as specifying *--current*.
+
+If *--requested-size* is specified then under memory target is
+changed to requested *size* (as scaled integer, see ``NOTES`` above). It
+defaults to kibibytes if no suffix is provided.
+
+
change-media
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 9746117bdb..0b32e6f408 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9128,6 +9128,154 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd)
return ret;
}
+
+/*
+ * "update-memory" command
+ */
+static const vshCmdInfo info_update_memory[] = {
+{.name = "help",
+ .data = N_("update memory device of a domain")
+},
+{.name = "desc",
+ .data = N_("Update values of a memory device of a domain")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_update_memory[] = {
+VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+VIRSH_COMMON_OPT_DOMAIN_CONFIG,
+VIRSH_COMMON_OPT_DOMAIN_LIVE,
+VIRSH_COMMON_OPT_DOMAIN_CURRENT,
+{.name = "print-xml",
+ .type = VSH_OT_BOOL,
+ .help = N_("print updated memory device XML instead of executing the
change")
+},
+{.name = "alias",
+ .type = VSH_OT_STRING,
+ .completer = virshDomainDeviceAliasCompleter,
+ .help = N_("memory device alias"),
+},
+{.name = "requested-size",
+ .type = VSH_OT_INT,
+ .help = N_("new value of size, as scaled integer (default
KiB)")
+},
+{.name = NULL}
+};
+
+static int
+virshGetUpdatedMemoryXML(char **updatedMemoryXML,
+ vshControl *ctl,
+ const vshCmd *cmd,
+ virDomainPtr dom,
+ unsigned int flags)
+{
+const char *alias = NULL;
+g_autoptr(xmlDoc) doc = NULL;
+g_autoptr(xmlXPathContext) ctxt = NULL;
+g_autofree char *xpath = NULL;
+int nmems;
+g_autofree xmlNodePtr *mems = NULL;
+g_autoptr(xmlBuffer) xmlbuf = NULL;
+unsigned int domainXMLFlags = 0;
+
+if (flags & VIR_DOMAIN_AFFECT_CONFIG)
+domainXMLFlags |= VIR_DOMAIN_XML_INACTIVE;
+
+if (virshDomainGetXMLFromDom(ctl, dom, domainXMLFlags, , ) < 0)
+return -1;
+
+if (vshCommandOptStringReq(ctl, cmd, "alias", ) < 0)
+return -1;
+
+if (alias) {
+xpath = g_strdup_printf("/domain/devices/memory[./alias/@name='%s']",
alias);
+} else {
+xpath = g_strdup("/domain/devices/memory");
+}
+
+nmems = virXPathNodeSet(xpath, ctxt, );
+if (nmems < 0) {
+vshSaveLibvirtError();
+return -1;
+} else if (nmems == 0) {
+vshError(ctl, _("no memory device found"));
+return -1;
+} else if (nmems > 1) {
+vshError(ctl, _("multiple memory devices found, use --alias to select
one"));
+return -1;
+}
+
+ctxt->node = mems[0];
+
+if (vshCommandOptBool(cmd, "requested-size")) {
+xmlNodePtr requestedSizeNode;
+g_autofree char *kibibytesStr = NULL;
+unsigned long long bytes = 0;
+unsigned long kibibytes = 0;
+
+if (vshCommandOptScaledInt(ctl, cmd, "requested-size", , 1024,
ULLONG_MAX) < 0)
+return -1;
+kibibytes = VIR_DIV_UP(bytes,
