Re: [PATCH 4/5] conf: Refactor virDomainDiskDefParseXML to pass vmdef

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

 



On Thu, Jul 16, 2015 at 11:24:33AM -0400, John Ferlan wrote:
> 
> 
> On 07/16/2015 08:03 AM, Ján Tomko wrote:
> > On Mon, Jun 22, 2015 at 05:05:06PM -0400, John Ferlan wrote:
> >> Rather than passing the def->seclabels and def->nseclabels, refactor
> >> the API to pass the entire domain definition.  This will be used in a
> >> future patch as well.
> > 
> > I think it would be nicer to separate XML parsing (which would just
> > record what was in the XML) and auto-generating missing configuration
> > (like generating drive addresses or checking for conflicts).
> > 
> > Jan
> > 
> 
> Are you advocating removing virDomainDiskDefAssignAddress() from
> virDomainDiskDefParseXML() and relying on qemuParseCommandLineDisk() and
> qemuParseCommandLine()?

Nobody should rely on qemuParseCommandLine, it is known to be
unreliable. :)

> 
> Following around all the existing "assumptions" especially as they
> relate to hotplug and even perhaps blockcopy could be an adventure. Not
> to say that I don't disagree with the concept separating parse from
> autofill; however, that seems outside the scope of this particular issue
> 

I meant moving the autofill logic to virDomainDefPostParseInternal
and virDomainDeviceDefPostParseInternal.

The latter is also called by the hotplug code. So the only assumptions
possibly broken would be during parsing of other devices - between disks/hostdev
parsing and calling the PostParse functions.

It seems the only affected function is virDomainDefMaybeAddHostdevSCSIcontroller
which can be moved to virDomainDefAddImplicitControllers.

Jan

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[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]