[libvirt] [PATCH v1 03/11] bandwidth: Create (un)plug functions

[Laine Stump]

On 12/03/2012 10:55 AM, Michal Privoznik wrote:
> On 30.11.2012 18:53, Laine Stump wrote:
>> As I'm looking at all these uses of id's, I'm wondering if there's
>> any danger of namespace conflict with other users of tc. (This isn't
>> any critique, just curiousity). 
> No, class ID is specific within an NIC. That is, classID of 3 on eth0
> doesn't interfere with class on vnet0. In fact, they have nothing in
> common. What we could interfere with is - if user sets something
> afterwards on fully libvirt managed interface. But I guess we don't
> support this, right? It's all or nothing approach to me.

Correct. I was only wondering if the use of the same class_id on a
different interface would cause interference. If it doesn't then nothing
to worry about :-)

>
>>> +{
>>> +    int ret = -1;
>>> +    int cmd_ret = 0;
>>> +    virCommandPtr cmd = NULL;
>>> +    char *class_id = NULL;
>>> +    char *qdisc_id = NULL;
>>> +    char *filter_id = NULL;
>>> +
>>> +    if (virAsprintf(&class_id, "1:%x", id) < 0 ||
>>> +        virAsprintf(&qdisc_id, "%x:", id) < 0 ||
>>> +        virAsprintf(&filter_id, "%u", id) < 0) {
>>> +        virReportOOMError();
>>> +        goto cleanup;
>>> +    }
>>> +
>>> +    cmd = virCommandNew(TC);
>>> +    virCommandAddArgList(cmd, "qdisc", "del", "dev", brname,
>>> +                         "handle", qdisc_id, NULL);
>>> +
>>> +    /* Don't threat tc errors as fatal, but
>>> +     * try to remove as much as possible */
>> What's your definition of "fatal"? In this case, if tc fails you return
>> -1, not 0.
> No, the return value of tc is stored into cmd_ret. Since we are not
> passing a NULL here, virCommandRun should fail if something went really
> wrong - e.g. OOM, or a pipe couldn't be created, and so on.

Right. I missed the presence of cmd_ret.