[libvirt] [PATCH v2 1/2] device_conf: Split virDomainDeviceInfoClear

[Andrea Bolognani]

On Tue, 2018-08-21 at 13:45 +0200, Michal Privoznik wrote:
> On 08/21/2018 01:15 PM, Andrea Bolognani wrote:
> > On Tue, 2018-08-21 at 12:12 +0200, Michal Privoznik wrote:
> > >  void
> > > -virDomainDeviceInfoClear(virDomainDeviceInfoPtr info)
> > > +virDomainDeviceInfoClearAddress(virDomainDeviceInfoPtr info)
> > >  {
> > > -    VIR_FREE(info->alias);
> > >      memset(&info->addr, 0, sizeof(info->addr));
> > >      info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
> > >      VIR_FREE(info->romfile);
> > 
> > Now virDomainDeviceInfoClearAddress() clears out *way more* than
> > just the address, including romfile and other information not
> > visible in the context... It should really only call memset and
> > reset info->type in order for the name not to be very misleading.
> > 
> > [...]
> > > +void
> > > +virDomainDeviceInfoClear(virDomainDeviceInfoPtr info)
> > > +{
> > > +    VIR_FREE(info->alias);
> > > +    virDomainDeviceInfoClearAddress(info);
> > > +}
> > 
> > romfile and friends should be cleared out here along with the alias
> > and, of course, the address :)
> 
> Actually, this is not going to work either. If somebody specifies:
> <master startport='1'/> into guestfwd XML we will parse it and format it
> back. The only solution I see is to have a flag to
> virDomainDeviceInfoParseXML() that parses just alias and nothing else.
> Which of course doesn't fit into the rest of the flags it already has.
> So the flag has to be the opposite: allow alias parsing and then fix all
> the places where it is called.
> 
> Sigh. This is gotten too far for such unusual corner case. I'm just
> going to close the bug as WONTFIX.
> 
> Thanks anyway.

You're right, I failed to consider that.

Given the situation I'd say that we should merge the initial fix
and err on the side of parsing/formatting information that doesn't
apply to the device rather than skipping information that does: the
former is suboptimal, but the latter is outright broken.

I'll ACK the v1 patch.

-- 
Andrea Bolognani / Red Hat / Virtualization