Kaitlin Rupert wrote:
Jay Gagnon wrote:
# HG changeset patch # User Jay Gagnon <grendel@linux.vnet.ibm.com> # Date 1208983463 14400 # Node ID 741b757ad68c5c4b35c5c697acbc121ac88d1ecf # Parent 2806f8744946757036f9639d8c8fe9a95f689233 [RFC] ProcessorRASD, the class that won't go away
Around and round we go on the merry-go-round of indecision, hopefully for our last ride. The definitely probably absolutely tentative final plan is that the default representation of processors as virt_device structs will be one per domain, with a quantity. Virt_Devices will just need to be told how to handle that, and all the RASD stuff will be much happier for it. And then we will never speak of this again.
=) Don't forget the corresponding change in xml_parse.c Ugh.
+static char *get_vcpu_inst_id(const virDomainPtr dom, + int proc_num) +{ + int rc; + char *id_num = NULL; + char *dev_id = NULL; + + rc = asprintf(&id_num, "%d", proc_num); + if (rc == -1) { + free(dev_id); + dev_id = NULL;
dev_id should already be NULL, right? The asprintf() doesn't attempt to modify it. That was my original assumption as well, but the asprintf() man page says: "If memory allocation wasn’t possible, or some other error occurs, these functions will return -1, and the contents of strp is undefined."
That "undefined" bit made me all paranoid and I really hate segfaults so I figured better safe than sorry.
+ + dev_id = get_vcpu_inst_id(dom, i);
If get_vcpu_inst_id() returns NULL, then the DeviceID gets set as NULL. The DeviceID is a key, and while it doesn't have to be unique, we do use it in a number of places.
Would it make more sense to return an error here? Returning an instance with a NULL DeviceID would make it difficult to call associations on it, I think.
Yea, at some point in the stack call that needs to turn into a real error. Just not sure at the moment if there's a particular point that makes more sense or not.
+ if (!instance)
I know this was already like this, but it should be:
if (instance == NULL)
Okay I can fix that.
list); + + if (!rc) + CU_DEBUG("Problem");
Since this is just an RFC - I'm guessing this is a place holder? Do you plan on returning an error here?
Yea, that was purely a "more important things to do" thing.
+ + cleanup_virt_devices(&devs, count);
out: free(devs); @@ -501,10 +554,13 @@ CMPIStatus get_device_by_name(const CMPI goto err; }
+#if 0 + /* TODO: Handle this one. */ instance = device_instance(broker, dev, dom, NAMESPACE(reference)); +#endif cleanup_virt_device(dev);
Usually I run the test site to test, but this causes the GetInstance() call to fail, which impacts a fair number of tests. But I did some hand testing that looked good.
Yea, sorry about that; this bit here puts things in a bit of a sad state of affairs. -- -Jay