[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