Re: [PATCH v2 3/5] igvm: move file load to complete callback

[Ani Sinha <anisinha@redhat.com>]

> On 11 Dec 2025, at 4:24 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> Add UserCreatableClass->complete callback function for igvm-cfg object.
> 
> Move file loading and parsing of the igvm file from the process function
> to the new complete() callback function.  Keep the igvm file loaded
> after processing, release it in finalize() instead, so we parse it only
> once.

LGTM.

> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Ani Sinha <anisinha@redhat.com>

> ---
> include/system/igvm-internal.h |  5 +++++
> backends/igvm-cfg.c            | 18 ++++++++++++++++++
> backends/igvm.c                |  9 ++++-----
> 3 files changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/include/system/igvm-internal.h b/include/system/igvm-internal.h
> index e0452080b127..d378d682b0fb 100644
> --- a/include/system/igvm-internal.h
> +++ b/include/system/igvm-internal.h
> @@ -12,6 +12,8 @@
> #include "qemu/typedefs.h"
> #include "qom/object.h"
> 
> +#include <igvm/igvm.h>
> +
> struct IgvmCfg {
>     ObjectClass parent_class;
> 
> @@ -21,7 +23,10 @@ struct IgvmCfg {
>      *           format.
>      */
>     char *filename;
> +    IgvmHandle file;
>     ResettableState reset_state;
> };
> 
> +IgvmHandle qigvm_file_init(char *filename, Error **errp);
> +
> #endif
> diff --git a/backends/igvm-cfg.c b/backends/igvm-cfg.c
> index e0df3eaa8efd..4014062e0f22 100644
> --- a/backends/igvm-cfg.c
> +++ b/backends/igvm-cfg.c
> @@ -53,6 +53,13 @@ static void igvm_reset_exit(Object *obj, ResetType type)
>     trace_igvm_reset_exit(type);
> }
> 
> +static void igvm_complete(UserCreatable *uc, Error **errp)
> +{
> +    IgvmCfg *igvm = IGVM_CFG(uc);
> +
> +    igvm->file = qigvm_file_init(igvm->filename, errp);
> +}
> +
> OBJECT_DEFINE_TYPE_WITH_INTERFACES(IgvmCfg, igvm_cfg, IGVM_CFG, OBJECT,
>                                    { TYPE_USER_CREATABLE },
>                                    { TYPE_RESETTABLE_INTERFACE },
> @@ -62,6 +69,7 @@ static void igvm_cfg_class_init(ObjectClass *oc, const void *data)
> {
>     IgvmCfgClass *igvmc = IGVM_CFG_CLASS(oc);
>     ResettableClass *rc = RESETTABLE_CLASS(oc);
> +    UserCreatableClass *uc = USER_CREATABLE_CLASS(oc);
> 
>     object_class_property_add_str(oc, "file", get_igvm, set_igvm);
>     object_class_property_set_description(oc, "file",
> @@ -73,14 +81,24 @@ static void igvm_cfg_class_init(ObjectClass *oc, const void *data)
>     rc->phases.enter = igvm_reset_enter;
>     rc->phases.hold = igvm_reset_hold;
>     rc->phases.exit = igvm_reset_exit;
> +
> +    uc->complete = igvm_complete;
> }
> 
> static void igvm_cfg_init(Object *obj)
> {
> +    IgvmCfg *igvm = IGVM_CFG(obj);
> +
> +    igvm->file = -1;
>     qemu_register_resettable(obj);
> }
> 
> static void igvm_cfg_finalize(Object *obj)
> {
> +    IgvmCfg *igvm = IGVM_CFG(obj);
> +
>     qemu_unregister_resettable(obj);
> +    if (igvm->file >= 0) {
> +        igvm_free(igvm->file);
> +    }
> }
> diff --git a/backends/igvm.c b/backends/igvm.c
> index fbb8300b6d01..a01e01a12a60 100644
> --- a/backends/igvm.c
> +++ b/backends/igvm.c
> @@ -869,7 +869,7 @@ static int qigvm_handle_policy(QIgvm *ctx, Error **errp)
>     return 0;
> }
> 
> -static IgvmHandle qigvm_file_init(char *filename, Error **errp)
> +IgvmHandle qigvm_file_init(char *filename, Error **errp)
> {
>     IgvmHandle igvm;
>     g_autofree uint8_t *buf = NULL;
> @@ -898,10 +898,11 @@ int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
>     QIgvm ctx;
> 
>     memset(&ctx, 0, sizeof(ctx));
> -    ctx.file = qigvm_file_init(cfg->filename, errp);
> -    if (ctx.file < 0) {
> +    if (cfg->file < 0) {
> +        error_setg(errp, "No IGVM file loaded.");
>         return -1;
>     }
> +    ctx.file = cfg->file;
> 
>     /*
>      * The ConfidentialGuestSupport object is optional and allows a confidential
> @@ -992,7 +993,5 @@ cleanup_parameters:
>     g_free(ctx.id_auth);
> 
> cleanup:
> -    igvm_free(ctx.file);
> -
>     return retval;
> }
> -- 
> 2.52.0
>