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@xxxxxxxxxx>
> > ---
> > 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.
