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

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).

> 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).