Re: [libvirt] [PATCH v2 8/8] domain: Keep assigned class_id in domstatus XML

On 11.12.2012 13:03, Laine Stump wrote: > On 12/04/2012 02:19 PM, Michal Privoznik wrote: >> Interfaces keeps a class_id, which is an ID from which bridge >> part of QoS settings is derived. We need to store class_id >> in domain status file, so we can later pass it to >> virNetDevBandwidthUnplug. >> --- >> src/conf/domain_conf.c | 13 +++++++++++++ >> 1 files changed, 13 insertions(+), 0 deletions(-) >> >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >> index 99aa08d..9f6d1e9 100644 >> --- a/src/conf/domain_conf.c >> +++ b/src/conf/domain_conf.c >> @@ -4777,6 +4777,17 @@ virDomainActualNetDefParseXML(xmlNodePtr node, >> hostdev, flags) < 0) { >> goto error; >> } >> + } else if (actual->type == VIR_DOMAIN_NET_TYPE_NETWORK) { >> + char *class_id = virXPathString("string(./class/@id)", ctxt); >> + if (class_id && >> + virStrToLong_ui(class_id, NULL, 10, &actual->class_id) < 0) { >> + virReportError(VIR_ERR_INTERNAL_ERROR, >> + _("Unable to parse class id '%s'"), >> + class_id); >> + VIR_FREE(class_id); >> + goto error; >> + } >> + VIR_FREE(class_id); >> } >> >> bandwidth_node = virXPathNode("./bandwidth", ctxt); >> @@ -12467,6 +12478,8 @@ virDomainActualNetDefFormat(virBufferPtr buf, >> break; >> >> case VIR_DOMAIN_NET_TYPE_NETWORK: >> + if (def->class_id) >> + virBufferAsprintf(buf, "<class id='%u'/>", def->class_id); >> break; >> default: >> virReportError(VIR_ERR_INTERNAL_ERROR, > I just added a comment to 6/8 - I think this patch should be done prior > to 6/8, and include the addition of class_id to the actualNetDef (well, > really I think that the class_id would be better added to > virNetDevBandwidth instead). > > ACK with class_id moved into the bandwidth object (unless there's some > reason you didn't do that), and a preference for re-ordering/re-grouping > as I mentioned in the previous paragraph. My motive is to keep virNetDevBandwidth clean, so it contains just bandwidth definition. Class ID should be part of network object itself. Something similar to domain definition and domain object. Definition should keep defined values, while an object should contain runtime info. In this case: Class ID is not something from user. It's rather generated piece of information which - moreover - is dependent on current implementation. I mean, it just keeps track of assigned ID so we can talk to /sbin/tc (=current impl).