Re: [PATCH 09/10] virsh: Introduce update-memory command

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <memory/> device. So far I just need to
> change <requested/> so I'm introducing --requested-size only; but
> the idea is that this is extensible for other cases too. For
> instance, want to change <myElement/>? Nnew --my-element
> argument can be easily introduced.
> 
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
>  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 ``<memory/>`` 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 ``<memory/>`` 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 ``<memory/>`` 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 ``<requested/>`` 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 <requested/> 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, &doc, &ctxt) < 0)
> +        return -1;
> +
> +    if (vshCommandOptStringReq(ctl, cmd, "alias", &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, &mems);
> +    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"));

So if you don't have useraliases, you can't use this for inactive XML
with 2 virtio mem?


> +        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", &bytes, 1024, ULLONG_MAX) < 0)
> +            return -1;
> +        kibibytes = VIR_DIV_UP(bytes, 1024);
> +
> +        requestedSizeNode = virXPathNode("./target/requested", ctxt);
> +
> +        if (!requestedSizeNode) {
> +            vshError(ctl, _("virtio-mem device is missing <requested/>"));


Here you mention virtio-mem.


> +            return -1;
> +        }
> +
> +        kibibytesStr = g_strdup_printf("%lu", kibibytes);
> +        xmlNodeSetContent(requestedSizeNode, BAD_CAST kibibytesStr);
> +    }
> +
> +    if (!(*updatedMemoryXML = virXMLNodeToString(doc, mems[0]))) {
> +        vshSaveLibvirtError();
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +static bool
> +cmdUpdateMemory(vshControl *ctl, const vshCmd *cmd)
> +{
> +    virDomainPtr dom;
> +    bool ret = false;
> +    bool config = vshCommandOptBool(cmd, "config");
> +    bool live = vshCommandOptBool(cmd, "live");
> +    bool current = vshCommandOptBool(cmd, "current");
> +    g_autofree char *updatedMemoryXML = NULL;
> +    unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
> +
> +    VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
> +    VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
> +
> +    if (config)
> +        flags |= VIR_DOMAIN_AFFECT_CONFIG;
> +    if (live)
> +        flags |= VIR_DOMAIN_AFFECT_LIVE;
> +
> +    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
> +        return false;
> +
> +    if (virshGetUpdatedMemoryXML(&updatedMemoryXML, ctl, cmd, dom, flags) < 0)
> +        goto cleanup;
> +
> +    if (vshCommandOptBool(cmd, "print-xml")) {
> +        vshPrint(ctl, "%s", updatedMemoryXML);
> +    } else {
> +        if (virDomainUpdateDeviceFlags(dom, updatedMemoryXML, flags) < 0)
> +            goto cleanup;
> +    }
> +
> +    ret = true;
> + cleanup:
> +    virshDomainFree(dom);
> +    return ret;

You can use g_autoptr(virshDomain) dom = NULL; on top instead of the
cleanup section.




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux