[libvirt] [PATCH] conf: fix cannot start a guest have a shareable network iscsi hostdev

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@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

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

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

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

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
participants (3)
-
lhuang
-
Luyao Huang
-
Michal Privoznik