8:16 a.m.
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
Since the previous discussions didn't really end up anywhere
conclusive
I decided it would be better to have a crack at getting some working code
to illustrate my ideas. Thus, the following series of 7 patches provide
Hi Dan,
Impressive work!
It's taken me a while just to digest all of this.
...
Some open questions
- Is it worth bothering with a UUID for individual storage volumes. It
is possible to construct a globally unique identifier for most volumes,
by combing various bits of metadata we have such as device unique ID,
inode, iSCSI target & LUN, etc There isn't really any UUID that fits
into the classic libvirt 16 byte UUID. I've implemented (randomly
generated) UUIDs for the virStorageVolPtr object, but I'm inclined
to remove them, since its not much use if they change each time the
libvirtd daemon is restarted.
The 'name' field provides a unique identifier scoped to the storage
pool. I think we could add a 'char *key' field, as an arbitrary opaque
string, forming a globally unique identifier for the volume. This
would serve same purpose as UUID, but without the 16 bytes constraint
which we can't usefully provide.
That sounds good. And with it being opaque, no one will
be tempted (or able) to rely on it.
- For the local directory backend, I've got the ability to
choose
between file formats on a per-volume basis. eg, /var/lib/xen/images can
contain a mix of raw, qcow, vmdk, etc files. This is nice & flexible
for the user, but a more complex thing to implement, since it means
we have to probe each volume and try & figure out its format each
Have there been requests for this feature?
The probe-and-recognize part doesn't sound too hard, but if
a large majority of use cases have homogeneous volumes-per-pool,
then for starters at least, maybe we can avoid the complexity.
A possible compromise (albeit ugly), _if_ we can dictate naming policy:
let part of a volume name (suffix, substring, component, whatever)
tell libvirtd its type. As I said, ugly, and hence probably not
worth considering, but I had to say it :-)
time we list volumes. If we constrained the choice between
formats
to be at the pool level instead of the volume level we could avoid
probing & thus simplify the code. This is what XenAPI does.
- If creating non-sparse files, it can take a very long time to do the
I/O to fill in the image. In virt-intsall/virt-manager we have nice
incremental progress display. The API I've got currently though is
blocking. This blocks the calling application. It also blocks the
entire libvirtd daemon since we are single process. There are a couple
of ways we can address this:
1 Allow the libvirtd daemon to serve each client connection in
a separate thread. We'd need to adding some mutex locking to the
QEMU driver and the storage driver to handle this. It would have
been nice to avoid threads, but I don't think we can much longer.
2 For long running operations, spawn off a worker thread (or
process) to perform the operation. Don't send a reply to the RPC
message, instead just put the client on a 'wait queue', and get
on with serving other clients. When the worker thread completes,
send the RPC reply to the original client.
3 Having the virStorageVolCreate() method return immediately,
giving back the client app some kind of 'job id'. The client app
can poll on another API virStorageVolJob() method to determine
how far through the task has got. The implementation in the
driver would have to spawn a worker thread to do the actual
long operation.
I like the idea of spawning off a thread for a very precise
and limited-scope task.
On first reading, I preferred your #2 worker-thread-based solution.
Then, client apps simply wait -- i.e., don't have to poll.
But we'd still need another interface for progress feedback, so #3
starts to look better: client progress feedback might come almost
for free, while polling to check for completion.
Possibly we can allow creation to be async or blocking by
making use of the 'flags' field to virStorageVolCreate() method,
eg VIR_STORAGE_ASYNC. If we make async behaviour optional, we
still need some work in the daemon to avoid blocking all clients.
This problem will also impact us when we add cloning of existing
volumes. It already sort of hits us when saving & restoring VMs
if they have large amounts of memory. So perhaps we need togo
for the general solution of making the daemon threaded per client
connection. The ASYNC flag may still be useful anyway to get the
incremental progress feedback in the UI.
Could we just treat that as another type of task to hand out to
a worker thread?
Otherwise, this (#1) sounds a lot more invasive, but that's just my
relatively uninformed impression.