[adding libvirt] On 09/18/2014 06:28 AM, Cole Robinson wrote:

- Say you are connecting from a new libvirt UNDEFINE_NVRAM support (say Fedora 21 GA), to an old libvirt without it, like F20 or RHEL7.0. If we specify the flag unconditionally, the undefineFlags call will fail, which also means that we lose the benefit of the other UNDEFINE flags. So if the VM had managedsave data, the undefine will fail saying we need to remove it first.

- Not all drivers support the UNDEFINE_NVRAM flag, like the 'test' driver which we use heavily for virt-manager development. If we pass the flag unconditionally, then the current code loses the benefit of all UNDEFINE flags, and trying to delete a VM with managedsave data will fail.

The existing code has similar problems as well though, since we lump all the UNDEFINE flags together, so it certainly isn't perfect. However my hack of checking for nvram in the XML avoids some of the common issues, so I'll extend it with the logic you pointed out.

Trying to figure out if an API or flag is supported with an arbitrary libvirt connection is a pain: it's a factor of host libvirt version, host libvirt-python version, remote libvirt version, libvirt hv driver, hv version. Some readonly APIs you can get away with just testing first, but undefine isn't one of them :) Maybe we can get a libvirt API exposing the driver function table at least?

Hmm, I've been thinking about that too. It would indeed be nice to introspect what APIs are available, and for those APIs with a flags parameter, what flag(s) bits are supported. What signature would we want? Thinking aloud here... Maybe: struct virAPI { const char *name; /* Name of the C API call */ unsigned int flags; /* The flag bits that are understood, if the C API includes a flags argument */ }; /* Allocate a list of supported API information into *list, and return the length of the list. User must call free() on the array when done. flags is 0 for now */ int virConnectListAPI(virConnectPtr conn, virAPIPtr *list, unsigned int flags); Or even having an enum mapping of C API names, instead of passing around char*. But the enum would not quite match the RPC call numbers. Implementation-wise, we've got a lot of driver calls that manually call virCheckFlags() at the front; maybe what we would do is modify the driver.h callback list to have a struct that contains both a callback pointer AND a flags value, rather than the current use of just a callback pointer; then we can make the rejection of unsupported flags in common code instead of manual virCheckFlags() everywhere, and we could also use the population of that registration as our point of documentation for when support for given bits in the flags argument is added. There's still the question of how dynamic this can be - for example, some qemu driver APIs accept a flag in name, but then fail if the underlying qemu binary is too old. Is it sufficient to document that the API knows about the flag, or do we really want the more complex solution of getting the exact subset of flags that have a chance of succeeding based on the underlying qemu binary? I'm leaning towards having the introspection API just report a flag if the driver plans on handling it; and in the cases where the underlying binary can cause the flag to succeed or fail, we can then use virConnectGetDomainCapabilities to advertise that sort of runtime difference (in other words, the API I'm thinking about adding is static and won't change answers based on a qemu upgrade; anything that depends on the actual running qemu affects is reflected through existing API). What about APIs that accept more than just a flags argument, such as the recent virConnectGetAllDomainStats, which as both a stats and flags parameter? Should the virAPI return struct contain a virTypedParameters or similar thing that can let us describe multiple details about a given API? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org