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

Re: [libvirt] [PATCH] rng: tighten up domain <controller> schema



On Thu, Apr 18, 2013 at 07:59:45AM -0400, Laine Stump wrote:
> On 04/18/2013 07:27 AM, Osier Yang wrote:
> > On 18/04/13 19:16, Laine Stump wrote:
> >> On 04/18/2013 05:41 AM, Martin Kletzander wrote:
> >>> On 04/18/2013 11:05 AM, Osier Yang wrote:
> >>>> On 18/04/13 17:00, Martin Kletzander wrote:
> >>>>> On 04/18/2013 10:54 AM, Osier Yang wrote:
> >>>>>> On 18/04/13 16:42, Martin Kletzander wrote:
> >>>>>>> On 04/18/2013 06:36 AM, Laine Stump wrote:
> >>>>>>>> The rng schema for <controller> had been non-specific about which
> >>>>>>>> types of controllers allowed which models, and also allowed the
> >>>>>>>> num_queues attribute (since that hasn't been released yet,
> >>>>>>>> should we
> >>>>>>>> rename it to "numQueues"?)
> >>>>>>> Since there's still time (the commit with that is
> >>>>>>> v1.0.4-65-gd4bf0a9), I
> >>>>>>> think we should rename it ASAP since we are using camelCase for
> >>>>>>> all the
> >>>>>>> attribute names.
> >>>>>>>
> >>>>>>> Apart from that, the RNG with this patch is precise according to
> >>>>>>> the
> >>>>>>> documentation, so ACK.  I'll try to send the numQueues patch to see
> >>>>>>> what
> >>>>>>> others think.
> >>>>>> I guess you mean multiple queues support for virtio network?
> >>>>>> Regardless of which style we will use finally, FYI,  "num_queues" is
> >>>>>> used for disk.. Personally I'm fine with either, because we already
> >>>>>> use both across.
> >>>>>>
> >>>>> Yes, I meant the virtio-scsi num_queues.  As we're trying not to use
> >>>>> underscores in XML, I hope we can still switch it.  I haven't
> >>>>> found any
> >>>>> other num_queues anywhere in the code.  Could you point me to the
> >>>>> commit
> >>>>> that uses that?  I'm sending the previously discussed patch in the
> >>>>> meantime.
> >>>>>
> >>>> Except the virtio-scsi num_queues, there is no other tag for multiple
> >>>> queue yet, we will need a patch to support multiple queue for the
> >>>> network,
> >>>> but it's not committed yet.
> >>>>
> >>>> It's fine to convert it now, 1.0.5 is not released yet. But is it
> >>>> deserved to
> >>>> do, we already have many tags with underscore, which can't be changed
> >>>> for back-compat.
> >>>>
> >>> I believe those attributes [1] were created by mistake, and kept only
> >>> because of backward compatibility.  I'm trying to be open-minded,
> >>> though, so I'm not forcing my patch in, but seeing it just as a
> >>> proposal.  Others may have different opinions and I'm willing to
> >>> discuss
> >>> that.  My first feeling, though, was that we should try to keep the
> >>> same
> >>> policy for as many of them as possible.  OTOH, I've mistaken the
> >>> underscore with a hyphen when I remembered what Daniel told me about
> >>> attributes [2].
> >> I had recalled DV saying something about underscores in the names a long
> >> time ago, and I recently asked about underscore vs. camelCase, and danpb
> >> said the same thing. (Personally I don't have a preference one way or
> >> the other, but if we really are trying to avoid them, now is our
> >> chance).

Yes, we should avoid underscores in all places.

> > I'm fine with either keeping it or changing num_queues. For long
> > term consistence, I agreed with having a consistent naming style
> > is nice.
> >
> >>
> >> In the meantime, in other device types, we've tried to keep backend
> >> details like this pushed into a <driver> subelement when possible, to
> >> avoid polluting the main element (e.g. see the <driver> subelement of
> >> <interface>). Is it worth putting this numQueues attribute in a <driver>
> >> subelement too? Or am I just playing XML God?
> >
> > Not sure if you mean the upcoming numQueues for interface. But for the
> > existing num_queues, it's for the virtio-scsi controller, putting it
> > in <driver>
> > doesn't reflect the purpose.
> 
> 
> But isn't it a backend implementation detail of the specific SCSI
> controller? In <interface> and <disk>, information that is specific to a
> particular backend (and isn't generally applicable to that type of
> device) is in the <driver> subelement.
> 
> (Just to confuse things a bit, it's actually the <driver> subelement
> where most of the underscored names live, and they were probably named
> with underscores for exactly the same reason you named num_queues -
> because that's the name used in qemu).

To be clear, QEMU's choice of underscores for naming its properties
has absolutely *ZERO* influence on libvirt's choice of naming. It is
a non-Goal to match QEMU's naming. So QEMU's use of underscores, does
not imply that libvirt should do the same.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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