[libvirt] Consultation of the patch "util: Prepare helpers for unpriv_sgio setting"

[yuxh]

On 03/13/2013 11:33 AM, Osier Yang wrote:
> On 2013年03月13日 11:04, yuxh wrote:
>> To Osier Yang
>>
>> Sorry for disturbing you on the working time, but I really need your 
>> help.
>> I met a problem when I do the virtualization test on the environment of
>> (Host: RHEL6.4GA. Guest: RHEL6.4GA) and it seems to have relationship
>> with your patch of "util: Prepare helpers for unpriv_sgio setting".
>
> Never mind, it's the bug I produced. :(
>
>>
>> I added a disk to virtio-scsi bus through the VM's XML file like this:
>> <disk type='block' device='disk'>
>> <driver name='qemu' type='raw' io='threads'/>
>> <source dev='/mnt/test.raw'/>
>> <target dev='sdb' bus='scsi'/>
>> <shareable/>
>> </disk>
>>
>> Then when I started the VM, I met the error.
>> [root at build SOURCES]# virsh start pan64ga
>> error: Failed to start domain pan64ga
>> error: Unable to get minor number of device '/mnt/test.raw': Invalid
>> argument
>>
>> [root at build SOURCES]#
>>
>> I tried other source files and other way of using if and I found:
>> 1). If we use the files which format is raw, qed and qcow2 as the disk
>> this error occur again.
>> 2). If we use partition(such as /dev/sdb1) or whole disk(such as
>> /dev/sdb) this error doesn't occur.
>> 3). If we delete the "<shareable/>" then files, partition and disk all
>> can be used correctly.
>>
>> And I investigate the error msg. it is returned by qemuGetSharedDiskKey
>> fuction.
>> char *
>> qemuGetSharedDiskKey(const char *disk_path)
>> {
>> ...
>> // error occur here. return value is -22
>> if ((rc = virGetDeviceID(disk_path, &maj, &min)) < 0) {
>> virReportSystemError(-rc,
>> _("Unable to get minor number of device '%s'"),
>> disk_path);
>> return NULL;
>> }
>> ...
>> }
>>
>> Go on the virGetDeviceID fuction:
>> int
>> virGetDeviceID(const char *path, int *maj, int *min)
>> {
>> ...
>>
>> if (!S_ISBLK(sb.st_mode)) {
>> VIR_FREE(canonical_path);
>> return -EINVAL; // error occur here and it return -22
>> }
>> ...
>> }
>>
>> So the error occur place is "S_ISBLK(sb.st_mode)" and I tried compiled
>> the libvirt with these lines deleted and the error didn't happen again.
>> Also I checked the disk if VM and it worked well.
>> I found this fuction virGetDeviceID is come from your patch of "util:
>> Prepare helpers for unpriv_sgio setting". But I am not familiar with
>> libvirt.
>> So I have to consult with you about whether this "S_ISBLK" check is
>> vital and If we can delete it or
>> find out some method to avoid this error so we can use the file as
>> shared disk in VM.
>
> It's fixd by 
> https://www.redhat.com/archives/libvir-list/2013-February/msg01024.html, 
> which is already committed to upstream, and backported
> to 6.4.z. You will see the fix in the new build, which will come soon.
>
> Regards,
> Osier
Sorry, I have tried the the libvirt's upstream code and it still have 
this error.
According to the code, if we use "<disk type='block' device='disk'> " 
and "<shareable/> " to set the disk's configuration in VM's XML file, it 
will go through like below:

int
qemuAddSharedDisk(virQEMUDriverPtr driver,
                   virDomainDiskDefPtr disk,
                   const char *name)
{  ...
     if (!(key = qemuGetSharedDiskKey(disk->src)))
         goto cleanup;
     ...
}

char *
qemuGetSharedDiskKey(const char *disk_path)
{
     ...
     if ((rc = virGetDeviceID(disk_path, &maj, &min)) < 0) {
         virReportSystemError(-rc,
                              _("Unable to get minor number of device 
'%s'"),
                              disk_path);
         return NULL;
     }
    ...
}

int
virGetDeviceID(const char *path, int *maj, int *min)
{
     struct stat sb;

     if (stat(path, &sb) < 0)
         return -errno;

     if (!S_ISBLK(sb.st_mode))
         return -EINVAL;

     if (maj)
         *maj = major(sb.st_rdev);
     if (min)
         *min = minor(sb.st_rdev);

     return 0;
}

To make a long story short, the virGetDeviceID  fuction want to get the 
maj and min number of the source file which we provided as the VM's 
disk. But the file such as /mnt/test.raw doesn't have the maj and min 
number.
So virGetDeviceID return " -EINVAL" and qemuGetSharedDiskKey print the 
error message of "Unable to get minor number of device /mnt/test.raw". 
That's the reason the VM can't be started.
So I just want to know now, if the way I used a file to act as VM's disk 
and set "<disk type='block' device='disk'>" , "<shareable/> " at the 
same time is reasonable.


Thanks
yuxh
>
>


-- 
以上

第一软件事业部 第一开发部 driver组 于星海
Best Regards
--------------------------------------------------
Yu Xinghai
Development Dept.I
Nanjing Fujitsu Nanda Software Tech. Co., Ltd.(FNST)
No.6 Wenzhu Road, Nanjing, 210012, China
TEL: +86+25-86630566-8533
FUJITSU INTERNAL: 7998-8533
FAX: +86+25-83317685
MAIL: yuxinghai at cn.fujitsu.com
--------------------------------------------------