[libvirt] Python setInterfaceParameters function broken

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); I am not sure if this is the best approach here. This change definitely fixes the issue, and allows setInterfaceParameters to actually execute successfully. I am not comfortable enough with my understanding of the issue to actually submit this as a patch though. Is this something that someone else can take a look at or fix? My trivial test script that was showing the issue: import libvirt conn = libvirt.open(None) if conn == None: print 'Failed to connect'; sys.exit(1) for id in conn.listDomainsID(): dom = conn.lookupByID(id) if dom == None: continue params = dom.interfaceParameters('vnet0', 0) print params dom.setInterfaceParameters('vnet0', params, 0)

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
participants (2)
-
Brian Rak
-
Michal Privoznik