Re: [libvirt] [PATCH 1/6] Add new API virDomainStreamDisk[Info] to header and drivers
On 05/09/2011 11:09 AM, Daniel P. Berrange wrote:
> int virDomainBlockStreamInfo(virDomainPtr dom,
> const char *device,
> virBlockStreamInfoPtr info);
Any reason you didn't just use the existing 'virDomainJobInfoPtr'
struct, with this new API ? In particular I think we want many
of the other fields from that struct beyond just 'remaining'.
I'm a bit sad to see that the libvirt API has diverged so much from the
QEMU API. I really see no reason to have a fancy mapping layer here.
We should pick and API that works for both of us and then implement it
in both layers.
In QEMU, we use an iterator based API. This is because we don't want
users providing specific offsets and sizes because, quite frankly,
there's no way they can choose good values. It's much more complicated
and far less efficient to stream in anything but the cluster size of the
image. And figuring this out is pretty much beyond even sophisticated
consumers.
So other than reporting progress as a fractional percentage, is there
really a compelling reason to not have an iterator API in libvirt? If
so, we should also revisit the QEMU side of this API.
Regards,
Anthony Liguori
Regards,
Daniel