Re: [libvirt PATCH 04/20] snapshot_conf: introduce <revertDisks> metadata element

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

 



On Mon, Apr 03, 2023 at 03:45:50PM +0200, Peter Krempa wrote:
> On Mon, Mar 13, 2023 at 16:42:05 +0100, Pavel Hrdina wrote:
> > This new element will hold the new disk overlay created when reverting
> > to non-leaf snapshot in order to remember the files libvirt created.
> > 
> > Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
> > ---
> >  src/conf/schemas/domainsnapshot.rng |  7 +++++++
> >  src/conf/snapshot_conf.c            | 27 +++++++++++++++++++++++++++
> >  src/conf/snapshot_conf.h            |  5 +++++
> >  3 files changed, 39 insertions(+)
> 
> [...]
> 
> > diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
> > index da1c694cb9..949104c1fd 100644
> > --- a/src/conf/snapshot_conf.c
> > +++ b/src/conf/snapshot_conf.c
> 
> [...]
> 
> > @@ -843,6 +859,17 @@ virDomainSnapshotDefFormatInternal(virBuffer *buf,
> >          virBufferAddLit(buf, "</disks>\n");
> >      }
> >  
> > +    if (def->nrevertdisks) {
> 
> Always use explicit comparison against 0 for non-pointer, non-booleans.
> 
> > +        virBufferAddLit(buf, "<revertDisks>\n");
> > +        virBufferAdjustIndent(buf, 2);
> > +        for (i = 0; i < def->nrevertdisks; i++) {
> > +            if (virDomainSnapshotDiskDefFormat(buf, &def->revertdisks[i], xmlopt) < 0)
> > +                return -1;
> > +        }
> > +        virBufferAdjustIndent(buf, -2);
> > +        virBufferAddLit(buf, "</revertDisks>\n");
> 
> Also preferrably use virXMLFormatElement.

I've basically copy&pasted what we do for the <disks> element. Will fix
it for this patch and post a separate patch fixing the other incorrect
comparisons and building xml.

> > +    }
> > +
> >      if (def->parent.dom) {
> >          if (virDomainDefFormatInternal(def->parent.dom, xmlopt,
> >                                         buf, domainflags) < 0)
> 
> Reviewed-by: Peter Krempa <pkrempa@xxxxxxxxxx>

Attachment: signature.asc
Description: PGP signature


[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