Re: [PATCH v1 01/12] libxl: add API wrapper for libxl_domain_create_restore

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

 



On Thu, Mar 18, 2021 at 09:51:18PM -0600, Jim Fehlig wrote:
> On 3/18/21 5:00 PM, Olaf Hering wrote:
> > Am Thu, 18 Mar 2021 16:26:14 -0600
> > schrieb Jim Fehlig <jfehlig@xxxxxxxx>:
> > 
> > > Maybe libxlDomainCreateRestoreWrap?
> > > The 'Wrap' suffix compliments the libxl_api_wrap.h name suggestion.
> > 
> > "Naming conventions" does not cover API wrapping.
> 
> I was referring to the use of '_' in the names. From the coding style doc:
> "Underscores should not be used in function names". The style doc doesn't
> dictate the words used to form function names, but does suggest a
> vir$object$verb$subject pattern.
> 
> > Some of the names are already taken, like libxl_domain_shutdown/libxlDomainShutdown.
> 
> In hindsight I would have probably used the 'vir' prefix in the driver entry
> points, e.g. virlibxlDomainShutdown (libxl_driver.c), giving some
> flexibility for driver-internal function naming. There is nothing preventing
> such change now, other than the future annoyance of backport conflicts.

FWIW, in retrospect, I think we shouldn't have used "libxl" as a naming
convention anywhere in libvirt - neither filenames or method names. This
is a Xen driver, and libxl is just an impl detail. IOW, I we ought to
have just use  "virXen" as the method name / typedef prefix, and
xen_driver.c  as filename, etc.  Obviously we avoided this originally
to distinguish the new impl from the old XenD, but I think that was
a mistake in retrospect, as we optimized for something that was only
going to exist for a few further years, as opposed to optimizing for
the long term where the libxl impl is the only one.

I don't feel strongly about whether you stick with current naming
conventions of change it to anything else - just wanted to throw
this out there as a option.

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