On Fri, Feb 20, 2015 at 12:43:50PM +0000, Daniel P. Berrange wrote: > On Fri, Feb 20, 2015 at 01:16:24PM +0100, Michal Privoznik wrote: > > On 20.02.2015 12:39, Ján Tomko wrote: > > > Doucment that not all attributes are used for matching. s/Doucment/Document/ > > > > > > https://bugzilla.redhat.com/show_bug.cgi?id=872028 > > > --- > > > src/libvirt-domain.c | 5 +++++ > > > tools/virsh.pod | 4 +++- > > > 2 files changed, 8 insertions(+), 1 deletion(-) > > > > > > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c > > > index 492e90a..a95c096 100644 > > > --- a/src/libvirt-domain.c > > > +++ b/src/libvirt-domain.c > > > @@ -8341,6 +8341,11 @@ virDomainDetachDevice(virDomainPtr domain, const char *xml) > > > * into S4 state (also known as hibernation) unless you also modify the > > > * persistent domain definition. > > > * > > > + * Note that not all attributes from the XML definition are checked. > > > + * For example, if the mac address and the pci address specified in the XML > > > + * match an existing interface, but source and interface type do not, > > > + * the existing interface will be detached. > > > + * > > > > I don't think we want to advertise this. Users are required to pass full > > device XML. If we use only a part of that information to find the > > device, it's our internal implementation. Not a green light for users to > > pass minimized XMLs. Should the docs explicity discourage minimized XMLs? virsh.pod already has this note: B<Note>: using of partial device definition XML files may lead to unexpected results as some fields may be autogenerated and thus match devices other than expected. > > Same applies for (partly) inconsistent XMLs (inconsistent to domain > > XML). If we now teach users it's okay to pass XML that differs to its > > domain counterpart, we are stuck with it forever. I'd await plenty of > > bug reports that we broke somebody's flow just because he was passing > > altered XML and we suddenly needed to change the set of attributes that > > are checked. Well, given enough users, every change breaks someone's workflow :) [1] Whether it's documented or not, I don't think we can suddenly start checking if the source path matches when detaching a cdrom device. But my intention was to explicitly document that the mentioned behavior is not a bug, not to commit to that behavior. Re-reading the patch, my wording was not ambiguous enough. > > > > NACK, sorry. > > Agreed, this docs change will limit our ability to change our impl > later if we need to. > Would you object to a more general wording, or should we just not advertise this at all? For example: The supplied XML description of the device should be as specific as its definition in the domain XML. The set of attributes used to match the device are internal to the drivers. Using a partial definition, or attempting to detach a device that is not present in the domain XML, but shares some specific attributes with one that is present may lead to unexpected results. Jan [1] https://xkcd.com/1172/
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list