Re: [libvirt] [PATCH] provide the object name on lookup error

On Tue, Jul 07, 2009 at 12:52:48PM -0400, Cole Robinson wrote: I don't really like the use of macros when flow control is involved because I think its important for people reading the code to clearly see code paths. I agree the amount of code involved here is getting a bit cumbersome though, particularly for UUID based errors. Part of the problem here is the tedious conversion from the raw UUID to printable UUID. This is causing us pain elsewhere - we hash virDomainPtr objects based on name since you can't use a raw UUID as a hash key. I think we should do a couple of things: * Change virDomainPtr internal struct to store the printable UUID in canonical format. This would allow us to remove all the char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(dom->uuid, uuidstr); * Add a macro around the qemuReportError() call, specifically for the VIR_ERR_NO_DOMAIN case, in same way we did for VIR_ERR_NO_MEMORY and VIR_ERR_SYSTEM_ERROR. #define virReportNoDomain(dom) \ virReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, _("no domain with matching name '%s' uuid '%s'"), dom->name, dom->uuid) This would make code to report an error look like if (!vm) { virReportNoDomain(dom); goto error; } * Change virConnectPtr/virGetDomain to do hashing based on dom->uuid, for better uniqueness which would avoid some problems we've seen in virt-manager with name based hashing. Further along, I'd also like to see us try to get some more generic helper methods for certain API operations. For example, LXC, QEMU and UML drivers all have very similar code for certain methods which could likely be pulled into a shared module CIL works on the intermediate files between the cpp and compile phase, so it doesn't care about macros from a functional POV. The only problem is human interpretation of the error location info it produces, since you'll get the line number of the macro call, not th eexact line within the macro - same problem you see in GDB when debugging code with macros. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|