On 27.06.2012 18:09, Hendrik Schwartke wrote:
> Thank you Michal and Eric for fast
>
> On 27.06.2012 17:22, Eric Blake wrote:
>> On 06/27/2012 09:14 AM, Michal Privoznik wrote:
>>> On 21.06.2012 16:58, Hendrik Schwartke wrote:
>>>> Introducing the attribute vendor_id to force the CPUID instruction
>>>> in a kvm guest to return the specified vendor.
>>>> ---
>>>> +<optional>
>>>> +<attribute name="vendor_id">
>>>> +<data type="string">
>>>> +<param name='pattern'>.{12}</param>
>> Rather than using '.', use [a-zA-Z...] to limit the RNG to the same set
>> of characters as the C code (I'm not sure what the official set is,
>> though). I can understand rejecting a too-long name, but do we really
>> also want to reject a too-short name?
> Ok, that right. I will fix the pattern. The reason for rejecting
> too-short names is that the CPUID instruction returns the vendor id in
> EBX, ECX and EDX, thus the vendor id must be exactly 12 characters long.
> So it would be necessary to pad short names in some way. I think the
> best way to handle them is to reject them.
>
>>
>>>> + def->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) {
>>>> +
>>>> + if(virXPathBoolean("boolean(./model[1]/@fallback)",
>>>> ctxt)) {
>> Style: space after 'if'
>>
>>
>>>> + }
>>>> +
>>>> + if(virXPathBoolean("boolean(./model[1]/@vendor_id)",
>>>> ctxt)) {
>>>> + char *vendor_id;
>>>> +
>>>> + vendor_id =
>>>> virXPathString("string(./model[1]/@vendor_id)", ctxt);
>>>> + if(!vendor_id ||
>>>> strlen(vendor_id)!=VIR_CPU_VENDOR_ID_LENGTH) {
>> two more instances. Also, space on both sides of '!='
>>
>>
>>>> + }
>>>> + for(i=0; i<strlen(vendor_id); i++) {
>> space after 'for'
>>
>>>> + if(!isprint(vendor_id[i]) ||
>>>> isspace(vendor_id[i])) {
>> and 'if'
>>
>>
>>>>
>>>> +#define VIR_CPU_VENDOR_ID_LENGTH 12
>>>> +
>>> Insert one space between '#' and define so this is indented properly.
>> 'make syntax-check' with 'cppi' installed will flag this for you.
> Thanks a lot, that's a very good hint!
>>
>>>> @@ -3924,6 +3926,8 @@ qemuBuildCpuArgStr(const struct qemud_driver
>>>> *driver,
>>>> goto cleanup;
>>>>
>>>> virBufferAdd(&buf, guest->model, -1);
>>>> + if(guest->vendor_id)
>> space after 'if'
>>
>>> Otherwise looking good. Can somebody chime in and provide more light on
>>> allowed characters in vendor id?
> Well, the Intel instruction manual doesn't make any statement about the
> allowed characters.
> There are two reasons why I check for printable characters and spaces.
> The first is that I think the CPUID instruction is intended that way.
> The second is that I want to prevent problems calling qemu. It's for
> example not possible to pass spaces in the vendor id to qemu.
>
>> Sorry, I can't help there. But I think we've pointed out enough issues
>> to warrant a v2 submission.
>>
> I will rework the patch and post it tomorrow.
>
According to Wikipedia, it's possible to have spaces in vendor ID:
http://en.wikipedia.org/wiki/CPUID#EAX.3D0:_Get_vendor_ID
e.g. "VIA VIA VIA ". In that case we need to find a way of telling this
to qemu. What about simple argument closing into quotation marks?
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list