Re: [PATCH v2 04/13] storage file: fill in src->dataFileStore during file probe

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

 



On Sat, Sep 07, 2024 at 17:15:23 +0300, Nikolai Barybin via Devel wrote:
> Signed-off-by: Nikolai Barybin <nikolai.barybin@xxxxxxxxxxxxx>
> ---
>  src/storage_file/storage_source.c | 28 ++++++++++++++++++++++++++++
>  src/storage_file/storage_source.h |  3 +++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/src/storage_file/storage_source.c b/src/storage_file/storage_source.c
> index 2cfe3bc325..20d5b30c6c 100644
> --- a/src/storage_file/storage_source.c
> +++ b/src/storage_file/storage_source.c
> @@ -543,6 +543,22 @@ virStorageSourceNewFromBacking(virStorageSource *parent,
>  }
>  
>  
> +int virStorageSourceNewFromDataFile(virStorageSource *parent,
> +                                    virStorageSource **dataFileSrc)

This file uses the new coding style for headers where type is on a
separate line. Please don't combine styles in this file.

This is also missing a comment describing what the function does as with
other functions in this file.

> +{
> +    int rc;
> +
> +    if ((rc = virStorageSourceNewFromChild(parent,
> +                                           parent->dataFileRaw,
> +                                           dataFileSrc)) < 0)
> +        return rc;
> +
> +    (*dataFileSrc)->format = VIR_STORAGE_FILE_RAW;
> +    (*dataFileSrc)->readonly = parent->readonly;

You can do these at the beginning and then return the value from
'virStorageSourceNewFromChild' directly without need for 'rc'

> +    return rc;
> +}
> +
> +
>  /**
>   * @src: disk source definition structure
>   * @fd: file descriptor
> @@ -1391,6 +1407,18 @@ virStorageSourceGetMetadataRecurse(virStorageSource *src,
>          }
>      }
>  
> +    if (src->dataFileRaw) {
> +        g_autoptr(virStorageSource) dataFileStore = NULL;
> +        if ((rv = virStorageSourceNewFromDataFile(src, &dataFileStore)) < 0)
> +            return -1;
> +
> +        /* the data file would not be usable for VM usage */
> +        if (rv == 1)
> +            return 0;
> +
> +        src->dataFileStore = g_steal_pointer(&dataFileStore);
> +    }
> +
>      if (src->backingStoreRaw) {
>          if ((rv = virStorageSourceNewFromBacking(src, &backingStore)) < 0)
>              return -1;
> diff --git a/src/storage_file/storage_source.h b/src/storage_file/storage_source.h
> index 63fefb6919..0514ff2364 100644
> --- a/src/storage_file/storage_source.h
> +++ b/src/storage_file/storage_source.h
> @@ -72,6 +72,9 @@ int
>  virStorageSourceNewFromBacking(virStorageSource *parent,
>                                 virStorageSource **backing);
>  
> +int virStorageSourceNewFromDataFile(virStorageSource *parent,
> +                                    virStorageSource **dataFileSrc);
> +

same here, please use the existing coding style

>  int
>  virStorageSourceGetRelativeBackingPath(virStorageSource *top,
>                                         virStorageSource *base,
> -- 
> 2.43.5
> 



[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