9:16 p.m.
https://bugzilla.redhat.com/show_bug.cgi?id=1174569
We should do nothing for the shareable network iscsi hostdev in
qemuAddSharedHostdev and qemuRemoveSharedHostdev. Shareable for
a network iscsi hostdev is not valid, so just ignore it.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/qemu/qemu_conf.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 8d3818e..9539231 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1158,7 +1158,8 @@ qemuAddSharedHostdev(virQEMUDriverPtr driver,
if (!hostdev->shareable ||
!(hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
- hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI))
+ hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI
&&
+ hostdev->source.subsys.u.scsi.protocol !=
VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI))
return 0;
if (!(key = qemuGetSharedHostdevKey(hostdev)))
@@ -1261,7 +1262,8 @@ qemuRemoveSharedHostdev(virQEMUDriverPtr driver,
if (!hostdev->shareable ||
!(hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
- hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI))
+ hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI
&&
+ hostdev->source.subsys.u.scsi.protocol !=
VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI))
return 0;
if (!(key = qemuGetSharedHostdevKey(hostdev)))
--
1.8.3.1
9:46 a.m.
On 16.12.2014 04:16, Luyao Huang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1174569
We should do nothing for the shareable network iscsi hostdev in
qemuAddSharedHostdev and qemuRemoveSharedHostdev. Shareable for
a network iscsi hostdev is not valid, so just ignore it.
If it is invalid, can't we just forbid it in the parsing phase?
Michal
4:04 a.m.
On 12/16/2014 11:46 PM, Michal Privoznik wrote:
On 16.12.2014 04:16, Luyao Huang wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1174569
>
> We should do nothing for the shareable network iscsi hostdev in
> qemuAddSharedHostdev and qemuRemoveSharedHostdev. Shareable for
> a network iscsi hostdev is not valid, so just ignore it.
If it is invalid, can't we just forbid it in the parsing phase?
Thanks for your review.
Maybe 'invalid' this words is not correct here, after i check the code
in qemuRemoveSharedHostdev
there are some words in there:
"Currently the only conflicts we have to care about for the shared disk
and shared host device is "sgio" setting, which is only valid for block
disk and scsi host device."
I think this means we should do nothing for the network iscsi hostdev
(just like what we do for the network disk, usb hostdev...). And i don't
think it means the <shareable/> is invalid for these disk, because they
are already 'shareable' in guests even libvirt do nothing.
But i cannot make sure i am right :)
Michal
Luyao
4:27 a.m.
On 17.12.2014 11:04, lhuang wrote:
On 12/16/2014 11:46 PM, Michal Privoznik wrote:
> On 16.12.2014 04:16, Luyao Huang wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1174569
>>
>> We should do nothing for the shareable network iscsi hostdev in
>> qemuAddSharedHostdev and qemuRemoveSharedHostdev. Shareable for
>> a network iscsi hostdev is not valid, so just ignore it.
>
> If it is invalid, can't we just forbid it in the parsing phase?
>
Thanks for your review.
Maybe 'invalid' this words is not correct here, after i check the code
in qemuRemoveSharedHostdev
there are some words in there:
"Currently the only conflicts we have to care about for the shared disk
and shared host device is "sgio" setting, which is only valid for block
disk and scsi host device."
I think this means we should do nothing for the network iscsi hostdev
(just like what we do for the network disk, usb hostdev...). And i don't
think it means the <shareable/> is invalid for these disk, because they
are already 'shareable' in guests even libvirt do nothing.
But i cannot make sure i am right :)
Okay, I've reworded the commit message and pushed.
ACK
Michal
6:37 a.m.
On 12/17/2014 06:27 PM, Michal Privoznik wrote:
On 17.12.2014 11:04, lhuang wrote:
>
> On 12/16/2014 11:46 PM, Michal Privoznik wrote:
>> On 16.12.2014 04:16, Luyao Huang wrote:
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1174569
>>>
>>> We should do nothing for the shareable network iscsi hostdev in
>>> qemuAddSharedHostdev and qemuRemoveSharedHostdev. Shareable for
>>> a network iscsi hostdev is not valid, so just ignore it.
>>
>> If it is invalid, can't we just forbid it in the parsing phase?
>>
> Thanks for your review.
>
> Maybe 'invalid' this words is not correct here, after i check the code
> in qemuRemoveSharedHostdev
> there are some words in there:
>
> "Currently the only conflicts we have to care about for the shared disk
> and shared host device is "sgio" setting, which is only valid for block
> disk and scsi host device."
>
> I think this means we should do nothing for the network iscsi hostdev
> (just like what we do for the network disk, usb hostdev...). And i don't
> think it means the <shareable/> is invalid for these disk, because they
> are already 'shareable' in guests even libvirt do nothing.
>
> But i cannot make sure i am right :)
Okay, I've reworded the commit message and pushed.
ACK
Thanks a lot for your review and help.
It is my fault i should write a clearly description when i sent this patch.
Michal
Luyao
3629
days inactive
3630
days old
4 comments
3 participants
participants (3)
-
lhuang -
Luyao Huang
-
Michal Privoznik