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

[Laine Stump]

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