[PATCH 03/13] qemu: blockjob: Store list of bitmaps disabled prior to commit

Peter Krempa pkrempa at redhat.com
Tue Mar 10 16:41:35 UTC 2020


On Thu, Mar 05, 2020 at 10:43:15 +0100, Pavel Mores wrote:
> On Wed, Mar 04, 2020 at 06:26:31PM +0100, Peter Krempa wrote:
> > Starting a commit job will require disabling bitmaps in the base image
> > so that they are not dirtied by the commit job. We need to store a list
> > of the bitmaps so that we can later re-enable them.
> > 
> > Add a field and status XML handling code as well as a test.
> > 
> > Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> > ---
> >  src/qemu/qemu_blockjob.h                      |  2 ++
> >  src/qemu/qemu_domain.c                        | 26 +++++++++++++++++++
> >  .../blockjob-blockdev-in.xml                  |  4 +++
> >  3 files changed, 32 insertions(+)
> > 
> > diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h
> > index 72c7fa053e..e2e28ca4d3 100644
> > --- a/src/qemu/qemu_blockjob.h
> > +++ b/src/qemu/qemu_blockjob.h
> > @@ -88,6 +88,8 @@ struct _qemuBlockJobCommitData {
> >      virStorageSourcePtr top;
> >      virStorageSourcePtr base;
> >      bool deleteCommittedImages;
> > +    char **disabledBitmapsBase; /* a NULL-terminated list of bitmap names which
> > +                                   were disabled in @base for the commit job */
> >  };
> 
> Is it guaranteed that the new member 'disabledBitmapsBase' is properly
> initialised?  How about something along the lines of
> 
> 
> diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
> index 2e53821d43..f9fc83430e 100644
> --- a/src/qemu/qemu_blockjob.c
> +++ b/src/qemu/qemu_blockjob.c
> @@ -312,6 +312,7 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm,
>      job->data.commit.top = top;
>      job->data.commit.base = base;
>      job->data.commit.deleteCommittedImages = delete_imgs;
> +    job->data.commit.disabledBitmapsBase = NULL;
>      job->jobflags = jobflags;
>  
>      if (qemuBlockJobRegister(job, vm, disk, true) < 0)
> 
> 
> Even if explicit initialisation is somehow not needed here, I'd consider that
> fact worth a line of explanation in the commit message.

g_new0 and VIR_ALLOC (which uses calloc) always zero-out the memory
which is allocated. This means that setting integers to 0, pointers to
NULL and booleans to false is always assumed when allocating a new
object and we rarely re-initialize it explicitly if ever.




More information about the libvir-list mailing list