Re: [PATCH 1/2] storage: change assigning qcow2 compat to 1.1 from 0.10 automatically
On Tue, 5 Mar 2024 at 14:00, Peter Krempa <pkrempa(a)redhat.com> wrote:
Just a note, please don't CC random people from the mailing list in the
libvirt project.
In projects with faster patch frequencies, I typically CC maintainers
for efficient communication.
However, considering there's no separate MAINTAINERS file assigning
each module to a maintainer in this context. Thanks for the clarification.
> On Tue, Mar 05, 2024 at 01:50:32 +0530, Abhiram Tilak wrote:
> > In the file `storage/storage_util.c` currently `compat` varible is begin
> > assigned to 0.10 by default. This patch changes this default value to 1.1.
> >
> > This is done in efforts to upgrade the default qcow2 image version to
> > 1.1.
>
> We prefer if the commit message justifies the change rather than just
> summarizes it.
>
> The justification can be something along:
>
> storage: Use modern qcow2 by default
>
> Change the default to modern qcow2 as it's supported by all qemu
> versions supported by libvirt and in fact 'qemu-img' already defaults to
> the new format for a long time.
>
>
> >
> > Resolves: https://gitlab.com/libvirt/libvirt/-/issues/602
> > Signed-off-by: Abhiram Tilak <atp.exp(a)gmail.com>
> > ---
> > src/storage/storage_util.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
> > index 7bf815d978..28d5fce4f0 100644
> > --- a/src/storage/storage_util.c
> > +++ b/src/storage/storage_util.c
> > @@ -765,7 +765,7 @@ storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDef
*encinfo,
> > if (info->compat)
> > virBufferAsprintf(&buf, "compat=%s,", info->compat);
> > else if (info->format == VIR_STORAGE_FILE_QCOW2)
> > - virBufferAddLit(&buf, "compat=0.10,");
> > + virBufferAddLit(&buf, "compat=1.1,");
> >
> > if (info->clusterSize > 0)
> > virBufferAsprintf(&buf, "cluster_size=%llu,",
info->clusterSize);
>
> Without all the changes in patch 2/2 this breaks the bulild. Our guides
> for sending patches state:
>
> "If you're going to submit multiple patches, the automated tests must
> pass after each patch, not just after the last one."
>
> https://libvirt.org/hacking.html#preparing-patches
> You can add:
>
> Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
>
> for v2 with the new commit message and the test changes squashed into
> patch 1 so that tests pass.
>