On Monday, 14 November 2016 22:43:55 CET Nitesh Konkar wrote:
> In the current scenario the controller parsing fails
> when no index is specified .As the docs point out
> that specifying index is optional, libvirt is expected
> to fill the index without erroring out.
>
> Signed-off-by: Nitesh Konkar <nitkon12@xxxxxxxxxxxxxxxxxx>
> ---
There's something I don't understand -- it looks to me there's a
mismatch between the test case you provide (and its output), and what
the code part for it actually does.
> Before the Patch:
> cat controller.xml
> <controller model="virtio-scsi" type="scsi" />
Your XML has no index="" attribute.
> virsh attach-device vm1 controller.xml --persistent
> error: Failed to attach device from controller.xml
> error: internal error: Cannot parse controller index -1
>
> After the patch:
> virsh attach-device vm1 controller.xml --persistent
> Device attached successfully
> ---
> src/conf/domain_conf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 6e008e2..c52e67f 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -8374,7 +8374,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
> if (idx) {
Here "idx" is non-null only if the attribute is actually found, which
does not seem the case in the XML snippet above.
> unsigned int idxVal;
> if (virStrToLong_ui(idx, NULL, 10, &idxVal) < 0 ||
> - idxVal > INT_MAX) {
> + idxVal > UINT_MAX) {
This looks wrong to me: UINT_MAX is the maximum value that is going to
fit into a 'unsigned int', so 'idxVal' will never be (even on error)
greater than that.
If the index must be non-negative, it looks like using virStrToLong_uip
instead of virStrToLong_ui could work fine in rejecting negative values
outright.
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Cannot parse controller index %s"), idx);
Considering this, I think the XML you used in your testsmost probably
has index="-1" as attribute -- can you please confirm?
Thanks,
--
Pino ToscanoAttachment:
signature.asc
Description: This is a digitally signed message part.
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list