[libvirt] Python setInterfaceParameters function broken
Michal Privoznik
mprivozn at redhat.com
Tue Mar 18 08:26:59 UTC 2014
On 17.03.2014 20:31, Brian Rak wrote:
> I've been trying to use domain.setInterfaceParameters, and I'm finding
> it's throwing the mysterious error:
>
> libvirt.libvirtError: argument unsupported: parameter '' not supported
>
> Enabling debug mode in libvirt reveals this:
>
> 2014-03-17 18:41:33.780+0000: 16927: debug :
> virDomainSetInterfaceParameters:8001 : dom=0x7f2ecc0009b0, (VM:
> name=SRVID8736, uuid=5121c1d6-19aa-4494-b6a7-09af270f24da),
> device=vnet0, params=0x7f2ecc0011f0, nparams=6, flags=0
> 2014-03-17 18:41:33.780+0000: 16927: debug :
> virDomainSetInterfaceParameters:8002 : params[""]=(uint)0
> 2014-03-17 18:41:33.780+0000: 16927: debug :
> virDomainSetInterfaceParameters:8002 : params[""]=(uint)64000
> 2014-03-17 18:41:33.780+0000: 16927: debug :
> virDomainSetInterfaceParameters:8002 : params[""]=(uint)10
> 2014-03-17 18:41:33.780+0000: 16927: debug :
> virDomainSetInterfaceParameters:8002 : params[""]=(uint)64000
> 2014-03-17 18:41:33.780+0000: 16927: debug :
> virDomainSetInterfaceParameters:8002 : params[""]=(uint)0
> 2014-03-17 18:41:33.780+0000: 16927: debug :
> virDomainSetInterfaceParameters:8002 : params[""]=(uint)0
> (irreverent lines trimmed)
>
> Which ultimately led me to setPyVirTypedParameter lines 200-202. These
> don't seem to work properly. I added some debugging:
>
> strncpy(temp->field, keystr, sizeof(*temp->field) - 1);
> temp->field[sizeof(*temp->field) - 1] = '\0';
> DEBUG("current key name in: %s, out: %s field size: %i\n",
> keystr, temp->field, sizeof(*temp->field) - 1);
> temp->type = params[i].type;
>
> which was displaying:
>
> current key name in: outbound.peak, out: field size: 0
> current key name in: inbound.peak, out: field size: 0
> current key name in: inbound.burst, out: field size: 0
> current key name in: inbound.average, out: field size: 0
> current key name in: outbound.average, out: field size: 0
> current key name in: outbound.burst, out: field size: 0
>
> I am not good enough in C to determine what exactly the correct fix is
> here. It seems like nothing that used this function could have ever
> worked. My trivial patch was:
>
> --- a/python27-libvirt/libvirt-python-1.2.2/libvirt-override.c
> +++ b/python27-libvirt/libvirt-python-1.2.2/libvirt-override.c
> @@ -197,8 +197,8 @@ setPyVirTypedParameter(PyObject *info,
> goto cleanup;
> }
>
> - strncpy(temp->field, keystr, sizeof(*temp->field) - 1);
> - temp->field[sizeof(*temp->field) - 1] = '\0';
> + strncpy(temp->field, keystr, VIR_TYPED_PARAM_FIELD_LENGTH - 1);
> + temp->field[VIR_TYPED_PARAM_FIELD_LENGTH - 1] = '\0';
> temp->type = params[i].type;
> VIR_FREE(keystr);
Yes, you're right. The problem is temp->field is a fixed-size array, so
dereferencing *temp->field addresses the first item which is size of 1B.
So we effectively copy nothing. The fix is either to discard the
dereference or use the constant. I suggest going with the latter as it's
more obvious. Moreover, there's no need for terminating the string: the
@temp is always within freshly allocated zero that is initially filled
with zeros. And I've noticed a memleak too.
I've proposed patch:
https://www.redhat.com/archives/libvir-list/2014-March/msg01055.html
Michal
More information about the libvir-list
mailing list