On Thu, Oct 05, 2017 at 10:44:29AM +0200, Michal Privoznik wrote:
> On 10/05/2017 10:10 AM, Daniel P. Berrange wrote:
> > On Wed, Oct 04, 2017 at 08:31:36AM +0200, Martin Kletzander wrote:
> >> On Tue, Oct 03, 2017 at 03:10:48PM +0100, Daniel P. Berrange wrote:
> >>> On Tue, Oct 03, 2017 at 04:03:20PM +0200, Martin Kletzander wrote:
> >>>> On Tue, Oct 03, 2017 at 02:53:46PM +0100, Daniel P. Berrange wrote:
> >>>>> On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin Kletzander wrote:
> >>>>>> On Tue, Oct 03, 2017 at 12:58:59PM +0200, Michal Privoznik wrote:
> >>>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1434451
> >>>>>>>
> >>>>>>> It comes handy for management application to be able to have a
> >>>>>>> per-device label so that it can uniquely identify devices it
> >>>>>>> cares about. The advantage of this approach is that we don't have
> >>>>>>> to generate aliases at define time (non trivial amount of work
> >>>>>>> and problems). The only thing we do is parse the user supplied
> >>>>>>> UUID and format it back. For instance:
> >>>>>>>
> >>>>>>>    <disk type='block' device='disk'>
> >>>>>>>      <driver name='qemu' type='raw'/>
> >>>>>>>      <source dev='/dev/HostVG/QEMUGuest1'/>
> >>>>>>>      <target dev='hda' bus='ide'/>
> >>>>>>>      <uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
> >>>>>>>      <address type='drive' controller='0' bus='0' target='0' 
> >>>>>>> unit='0'/>
> >>>>>>>    </disk>
> >>>>>>>
> >>>>>>> Signed-off-by: Michal Privoznik <mpriv...@redhat.com>
> >>>>>>> ---
> >>>>>>>
> >>>>>>> This is just a very basic implementation. If I get a green light on 
> >>>>>>> this, I can
> >>>>>>> implement the feature further, i.e. allow device lookup on the UUID. 
> >>>>>>> For
> >>>>>>> instance:
> >>>>>>>
> >>>>>>> virsh domiftune fedora $UUID $bandwidth
> >>>>>
> >>>>> <snip>
> >>>>>
> >>>>> I'm thinking that part of the problem we're having with agreeing how to
> >>>>> deal with this RFE is that we're over-analysing semantics, by wondering
> >>>>> whether its a unique name or UUID, its relation to alias, whether it has
> >>>>> bearing on APIs.
> >>>>>
> >>>>> How about we change tack, and do what we did when we needed application
> >>>>> specific information at the top level - just declare a general purpose
> >>>>> <metadata> element and say it is a completely opaque blob. Libvirt will
> >>>>> *never* do anything with it, other than to preserve it exactly as is.
> >>>>> No API will ever use the metadata in any way. Libvirt will never try to
> >>>>> guarantee uniqueness of metadata for each device. It can be JSON or a
> >>>>> gziped microsoft word document for all we care. Entirely upto the app
> >>>>> developer to decide what metadata is saved and guarantee uniqueness if
> >>>>> they so desired.
> >>>>>
> >>>>
> >>>> That is kind of what I was aiming for.  But in order for it to be 
> >>>> cleaner and
> >>>> easier to use from user as well (and not only mgmt apps) I thought the 
> >>>> metadata
> >>>> would just be one identifier.  If you want to store more metadata for the
> >>>> device, then you can do all that in the domain metadata and just 
> >>>> reference the
> >>>> particular device using the identifier if mgmt app wants to do that.
> >>>
> >>> Yes that is certainly possible. The caveats are that we still need a 
> >>> unique
> >>> identifier for the device, and the metadata update is not atomic wrt
> >>> to device hotplug.
> >>>
> >>
> >> Yes, well, our (libvirt) unique identifier is not going anywhere, so
> >> that's OK, we'll be using what we have been until now.
> >>
> >>> The plus side of the global metadata is that we have APIs to update it
> >>> on the fly already, and its fully namespaced to allow multiple independant
> >>> data sets to be stored.
> >>>
> >>
> >> Yes, exactly.
> >>
> >>> I don't think lack of atomicity is a big deal as you could order it so 
> >>> that
> >>> you update metadata before doing the hotplug. Then worst case you have a
> >>> device mentioned in metadata that doesn't exist, which is easy enough to
> >>> detect.
> >>>
> >>
> >> Right, if you want metadata for device, then you'll just update
> >> metadata, hotplug device, and if it failed you update the metadata once
> >> more.
> >>
> >> So are we on the same page?  By that I mean agreeing on any sane 
> >> user-supplied
> >> identifier that we'll not guarantee uniqueness for, and neither will we 
> >> use it
> >> for anything for now?  (We can deal with the issues regarding using it when
> >> someone wants to actually implement it).
> > 
> > Per my reply to the earlier patch series, I'm now inclined to say that we
> > should
> > 
> >  - Allow the mgmt app to set the aliases upfront
> >  - Auto-fill missing aliases at XML define time
> > 
> > it has some downsides, but all the other solutions we've discussed have
> > their own downsides too. So on balance I think its not worth it to add
> > a second identifier for each device, when we already have alias.
> 
> Question is if we are confident enough that:
> 
> a) apps will provide unique aliases (since we'll be putting user input
> onto qemu cmd line)
> 
> b) apps will provide only allowed characters in the alias (not every
> character can be in id=, can it?)

We will have to validate both these points when looking at the XML.

> But I think we still have not answered this question: what if we need to
> change pattern by which we generate aliases in the future? On one hand,
> an alias is just a string so the pattern should not matter. On the other
> hand, that's not quite true. For instance, "pci.0" has a very special
> meaning. IOW, if we now worry about users cutting off the branch they
> are sitting on, this is like giving them flamethrower in fireworks
> production hall.

'pci.0' is not an alias - 'pci' is the alias, the '0' is a bus number,
so users only provide the first bit which has no special semantics
other than needing to comply with a permitted set of characters and
be unique. 

In terms of validation I think we should permit  a-Z, 0-9 and -, upto
a maximum of say 32 characters in length.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

Reply via email to