[libvirt] [PATCH] lib: fix no zero arg check for iothread_id
https://bugzilla.redhat.com/show_bug.cgi?id=1251886 We do not allow delete an iothread which id is 0 in virDomainDelIOThread, but allow it in virDomainAddIOThread, Also we will output an error when parse an iothread which id is 0. Add a check for iothread_id in virDomainAddIOThread to fix it. Signed-off-by: Luyao Huang <lhuang@redhat.com> --- Is that okay to fix it in this place ? src/libvirt-domain.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 837933f..8506942 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -7987,6 +7987,7 @@ virDomainAddIOThread(virDomainPtr domain, virCheckDomainReturn(domain, -1); virCheckReadOnlyGoto(domain->conn->flags, error); + virCheckNonZeroArgGoto(iothread_id, error); conn = domain->conn; -- 1.8.3.1
On Mon, Aug 10, 2015 at 17:06:31 +0800, Luyao Huang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1251886
We do not allow delete an iothread which id is 0 in virDomainDelIOThread, but allow it in virDomainAddIOThread, Also we will output an error when parse an iothread which id is 0.
Add a check for iothread_id in virDomainAddIOThread to fix it.
The limitation that iothread id shall not be 0 comes from the qemu implementation so I think that we could possibly want to have iothread id 0 in the future. I think the check should be done in the qemu driver. Peter
On 08/10/2015 05:23 PM, Peter Krempa wrote:
On Mon, Aug 10, 2015 at 17:06:31 +0800, Luyao Huang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1251886
We do not allow delete an iothread which id is 0 in virDomainDelIOThread, but allow it in virDomainAddIOThread, Also we will output an error when parse an iothread which id is 0.
Add a check for iothread_id in virDomainAddIOThread to fix it. The limitation that iothread id shall not be 0 comes from the qemu implementation so I think that we could possibly want to have iothread id 0 in the future.
I think the check should be done in the qemu driver.
Make sense, i will move the check in qemu driver, also i will change the code for iothread pin/delete Thanks for your quick review.
Peter
Luyao
On 08/10/2015 05:23 AM, Peter Krempa wrote:
On Mon, Aug 10, 2015 at 17:06:31 +0800, Luyao Huang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1251886
We do not allow delete an iothread which id is 0 in virDomainDelIOThread, but allow it in virDomainAddIOThread, Also we will output an error when parse an iothread which id is 0.
Add a check for iothread_id in virDomainAddIOThread to fix it.
The limitation that iothread id shall not be 0 comes from the qemu implementation so I think that we could possibly want to have iothread id 0 in the future.
Sort of.. The initial implementation used 0 (zero) as a marker of sorts for no iothreads, so 'iothread0' made no sense. At that time there was no add/del a specific thread. However, now that there are iothreadid's to manage the iothreads<N> value, it perhaps makes less sense to restrict usage of 0 (zero) except of course for historical purposes. There probably would need to be checks in multiple places adjusted though in order to manage the iothreadid's which 'assume' we start at 1 and count upwards when there is no <iothread id="#"/> provided in the domain XML. John
I think the check should be done in the qemu driver.
Peter
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 08/10/2015 09:17 PM, John Ferlan wrote:
On 08/10/2015 05:23 AM, Peter Krempa wrote:
On Mon, Aug 10, 2015 at 17:06:31 +0800, Luyao Huang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1251886
We do not allow delete an iothread which id is 0 in virDomainDelIOThread, but allow it in virDomainAddIOThread, Also we will output an error when parse an iothread which id is 0.
Add a check for iothread_id in virDomainAddIOThread to fix it. The limitation that iothread id shall not be 0 comes from the qemu implementation so I think that we could possibly want to have iothread id 0 in the future.
Sort of.. The initial implementation used 0 (zero) as a marker of sorts for no iothreads, so 'iothread0' made no sense. At that time there was no add/del a specific thread.
However, now that there are iothreadid's to manage the iothreads<N> value, it perhaps makes less sense to restrict usage of 0 (zero) except of course for historical purposes. There probably would need to be checks in multiple places adjusted though in order to manage the iothreadid's which 'assume' we start at 1 and count upwards when there is no <iothread id="#"/> provided in the domain XML.
Oh, i see, that is why i cannot find any reason to forbid iothread0 in qemu source code, thanks a lot for clearly explanation. Indeed, if we allow 0 as a valid iothread id and start at 0 will need some work to fix the possible issue happen on a guest with iothread during update, IMHO: it worth to do that :) Thanks a lot for your reply
John
Luyao
I think the check should be done in the qemu driver.
Peter
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (4)
-
John Ferlan -
lhuang -
Luyao Huang
-
Peter Krempa