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

Re: [libvirt] [PATCH] cleanup virDomainCreateLinux into virDomainDefineXML



On Thu, Oct 09, 2008 at 09:32:47PM +0100, Daniel P. Berrange wrote:
> On Thu, Oct 09, 2008 at 05:16:37PM +0200, Daniel Veillard wrote:
> >   As promised in the libvirt-qpid thread, virDomainCreateLinux() call
> > name makes no sense (anymore), and it should be renamed, i guess since
> > virDomainCreate() already exist and by similarity with
> > virDomainDefineXML() the best name is virDomainCreateXML().
> 
> Agreed - we already have virNetworkCreateXML(), so virDomainCreateXML()
> makes perfect sense.
> 
> >   The associated patch rename virDomainCreateLinux to virDomainCreateXML
> > create a small function virDomainCreateLinux calling the former,
> > document it as deprecated. To help comprehension of the source code
> > it's also best to rename the internal driver method in a similar
> > way, which inflates the patch a bit but is IMHO worth it.
> 
> Since this doesn't impact the on-the-wire RPC number, this seems
> like a worthwhile change too.
> 
> > >   The patch also fixes a few #elif define(__sun__) and turn them
> > into the correct #elif defined(__sun__) cpp instructions, and changes
> > include/libvirt/virterror.h to improve the generated HTML page about the
> > deprected fileds in the structure, since we need to regenerate the
> > API xml and the docs, it was a good opportunity for that small change.
> > 
> >   I removed the docs/ subdir part from the patch as it's generated
> > and mostly unreadable, that keeps it smaller too.
> 
> ACK to this all with just a few minor nit-picks.

  Okay, I'm pushing this to CVS now !

> > +/**
> > + * virDomainCreateLinux:
> > + * @conn: pointer to the hypervisor connection
> > + * @xmlDesc: string containing an XML description of the domain
> > + * @flags: callers should always pass 0
> > + *
> > + * Deprecated after 0.4.6 use virDomainCreateXML()
> 
> Can we make this a little clear that they provide 100% functionally
> identical impl. Something like
> 
>    * Deprecated after 0.4.6.
>    * Renamed to virDomainCreateXML() providing identical functionality.
>    * This existing name will left indefinitely for API compatability.
  
  Sure, done !

> All the files in qemud/ named  remote_XXXX  are automatically generated
> from remote_protocol.x, so if you change that one file the others should
> update. I don't see a change to remote_protocol.x in this diff, so don;t
> forget to update it when committing, or next protocol update will revert
> your changes by mistake.

  Right, I knew there was still something fishy. Also the perl script
generate CreateXml, not CreateXML, so fixed this (but the counter part
in src/ which is not generated use the CreateXML entry point :-)

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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