On Tue, Oct 11, 2022 at 21:38:27 +0800, Jiang Jiacheng wrote:
> Introduce qemuDomainChangeBootIndex api to support update device's bootindex.
> These function will be used in following patches to support change device's
> (support cdrom, disk and net) bootindex with virsh command like
> 'virsh update-device <domain> <file> [--flag]'.
> 
> Signed-off-by: Jiang Jiacheng <jiangjiach...@huawei.com>
> ---
>  src/qemu/qemu_conf.c         | 24 ++++++++++++++++++++++++
>  src/qemu/qemu_conf.h         |  5 +++++
>  src/qemu/qemu_monitor.c      | 12 ++++++++++++
>  src/qemu/qemu_monitor.h      |  6 ++++++
>  src/qemu/qemu_monitor_json.c | 22 ++++++++++++++++++++++
>  src/qemu/qemu_monitor_json.h |  6 ++++++
>  6 files changed, 75 insertions(+)
> 
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 3b75cdeb95..b653f44f25 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1571,3 +1571,27 @@ qemuGetMemoryBackingPath(virQEMUDriver *driver,
>      *memPath = g_strdup_printf("%s/%s", domainPath, alias);
>      return 0;
>  }
> +

Please document the expectations of the API ... e.g. the values that the
'newBootIndex' variable takes.

> +int
> +qemuDomainChangeBootIndex(virDomainObj *vm,
> +                          virDomainDeviceInfo *devInfo,
> +                          int newBootIndex)
> +{
> +    int ret = -1;
> +    qemuDomainObjPrivate *priv = vm->privateData;
> +
> +    if (!devInfo->alias) {
> +        virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> +                       _("cannot change boot index: device alias not 
> found"));
> +        return -1;
> +    }
> +
> +    VIR_DEBUG("Change dev: %s boot index from %d to %d", devInfo->alias,
> +              devInfo->bootIndex, newBootIndex);
> +
> +    qemuDomainObjEnterMonitor(vm);
> +    ret = qemuMonitorSetBootIndex(priv->mon, devInfo->alias, newBootIndex);

So here you pass alias as qom-path for qom-set. I'm not sure what our
policy on using the shortcut path is though as all code using
qom-set/qom-get always uses the full absolute path.

> +    qemuDomainObjExitMonitor(vm);
> +
> +    return ret;
> +}
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index c40c452f58..4e2cd84fe5 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -359,3 +359,8 @@ int qemuGetMemoryBackingPath(virQEMUDriver *driver,
>                               const virDomainDef *def,
>                               const char *alias,
>                               char **memPath);
> +
> +int qemuDomainChangeBootIndex(virDomainObj *vm,
> +                              virDomainDeviceInfo *devInfo,
> +                              int newBootIndex)
> +    ATTRIBUTE_NONNULL(2);
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index c2808c75a3..1b120d766d 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -4417,3 +4417,15 @@ qemuMonitorExtractQueryStats(virJSONValue *info)
>  
>      return g_steal_pointer(&hash_table);
>  }
> +
> +int
> +qemuMonitorSetBootIndex(qemuMonitor *mon,
> +                        const char *name,
> +                        int bootIndex)
> +{
> +    VIR_DEBUG("name=%s, bootIndex=%d", name, bootIndex);
> +
> +    QEMU_CHECK_MONITOR(mon);
> +
> +    return qemuMonitorJSONSetBootIndex(mon, name, bootIndex);
> +}
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 63269e15bc..f27431a3f2 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -1529,3 +1529,9 @@ qemuMonitorQueryStats(qemuMonitor *mon,
>  
>  GHashTable *
>  qemuMonitorExtractQueryStats(virJSONValue *info);
> +
> +int
> +qemuMonitorSetBootIndex(qemuMonitor *mon,
> +                        const char *name,
> +                        int bootIndex)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 70fba50e6c..5cc1f2665a 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -8665,3 +8665,25 @@ qemuMonitorJSONQueryStats(qemuMonitor *mon,
>  
>      return virJSONValueObjectStealArray(reply, "return");
>  }
> +
> +int
> +qemuMonitorJSONSetBootIndex(qemuMonitor *mon,
> +                            const char *name,
> +                            int bootIndex)
> +{
> +    g_autoptr(virJSONValue) cmd = NULL;
> +    g_autoptr(virJSONValue) reply = NULL;
> +
> +    if (!(cmd = qemuMonitorJSONMakeCommand("qom-set", "s:path", name,
> +                                           "s:property", "bootindex",

Note that you use 'name' as parameter, but in fact it's a qom path.
Please rename it appropriately to avoid confusion.

> +                                           "i:value", bootIndex, NULL)))
> +        return -1;
> +
> +    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
> +        return -1;
> +
> +    if (qemuMonitorJSONCheckError(cmd, reply) < 0)
> +        return -1;
> +
> +    return 0;
> +}

Reply via email to