Re: [PATCH 15/36] datatypes: convert virDomainCheckpoint to GObject

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

 



On Mon, Apr 06, 2020 at 01:32:39PM +0200, Rafael Fonseca wrote:
> On Mon, Apr 6, 2020 at 11:05 AM Daniel P. Berrangé <berrange@xxxxxxxxxx> wrote:
> >
> > On Mon, Apr 06, 2020 at 09:50:59AM +0200, Rafael Fonseca wrote:
> > > On Fri, Apr 3, 2020 at 5:16 PM Rafael Fonseca <r4f4rfs@xxxxxxxxx> wrote:
> > > >
> > > > +#define VIR_TYPE_DOMAIN_CHECKPOINT vir_domain_checkpoint_get_type()
> > > > +G_DECLARE_FINAL_TYPE(virDomainCheckpoint,
> > > > +                     vir_domain_checkpoint,
> > > > +                     VIR,
> > > > +                     DOMAIN_CHECKPOINT,
> > > > +                     GObject);
> > > > +
> > > >  extern virClassPtr virAdmConnectClass;
> > > >
> > > >  #define VIR_TYPE_ADM_SERVER vir_adm_server_get_type()
> > > > @@ -327,8 +333,8 @@ G_DECLARE_FINAL_TYPE(virAdmClient, vir_adm_client, VIR, ADM_CLIENT, GObject);
> > > >
> > > >  #define virCheckDomainCheckpointReturn(obj, retval) \
> > > >      do { \
> > > > -        virDomainCheckpointPtr _check = (obj); \
> > > > -        if (!virObjectIsClass(_check, virDomainCheckpointClass) || \
> > > > +        virDomainCheckpointPtr _check = VIR_DOMAIN_CHECKPOINT(obj); \
> > > > +        if (!G_IS_OBJECT(_check) || !(G_OBJECT_TYPE(_check) == VIR_TYPE_DOMAIN_CHECKPOINT) || \
> > >
> > > I guess `VIR_IS_DOMAIN_CHECKPOINT` created by `G_DECLARE_FINAL_TYPE`
> > > is enough here for this check?
> >
> > Yes, we can slim this right now and avoid the casts / intermediate
> > variable entirely, to just this I think:
> >
> >    #define virCheckDomainCheckpointReturn(obj, retval) \
> >       if (!VIR_IS_DOMAIN_CHECKPOINT(obj)) { \
> >         return retval; \
> >       }
> 
> That's much better. I'll do it for v2.
> 
> Another issue with the changes in datatypes.c is that by converting
> the types separately, then `gendispatch.pl` generates wrong code
> (virObjectUnref for GObject). Is it acceptable to have them all
> changed in a single patch?

I think having one patch will be too large. We'll need to update
gendispatch.pl so it knows how to generate a different method
name depending on object type.

This shouldn't be too hard - eg have a global hash

  my %unrefimpl = (
     "virDomain" => "virObjectUnref",
     "virNetwork" => "virObjectUnref",
     "virStoragePool" => "virObjectUnref",
     etc
  )

when just change them one at a time.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|





[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