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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
