Re: [libvirt] [PATCH v5 1/5] Introduce virDomainFSFreeze() and virDomainFSThaw() public API

Hi Daniel, thanks for your comment. On 4/22/14 11:39 , "Daniel P. Berrange" <berrange(a)redhat.com&gt; wrote: >On Thu, Apr 03, 2014 at 11:39:29AM -0400, Tomoki Sekiyama wrote: >> + >> +/** >> + * virDomainFSFreeze: >> + * @dom: a domain object >> + * @disks: list of disk names to be frozen >> + * @ndisks: the number of disks specified in @disks > >I realize this current patch series doesn't use the @disks parameter >at all, but what exactly are we expecting to be passed here ? Is >this a list of filesystem paths from the guest OS pov, or is it a >list of disks targets from libvirt's POV ? I'm guessing the former >since this is expected to be passed to the guest agent. My intention for 'disks' is latter, a list of disks targets from libvirt's POV. Currently it is just a placeholder of API. It would be passed to the agent after it is converted into a list of device addresses (e.g. a pair of drive address and controller's PCI address) so that the agent can look up filesystems on the specified disks.

>> + * @flags: extra flags, not used yet, so callers should always pass 0 >> + * >> + * Freeze filesystems on the specified disks within the guest (hence >>guest >> + * agent may be required depending on hypervisor used). If @ndisks is >>0, >> + * every mounted filesystem on the guest is frozen. >> + * Freeze can be nested. When it is called on the same disk multiple >>times, >> + * the disk will not be thawed until virDomainFSThaw is called the >>same times. > >I'm not entirely convinced we should allow nesting. I think it would >be better to report an error if the user tries to freeze a disk that >is already frozen, or tries to thaw a disks that is not frozen. Nesting >makes it harder for applications to know just what state the guest is >in. > >eg, consider that they need to run a command in the guest agent that >requires the guest FS to be un-thawed. If we allow nesting, then after >the app runs virDomainFSThaw, the app can't tell whether or not all >thes path are un-thawed or not. > >IMHO we should forbid nesting. Nesting was originally advised by Eric for the reason of scalability. But if it is not actually wanted by apps, I think we can remove nesting from API design. (I personally don't know apps that run parallel FSFreeze operations for multiple disks.) From the implementation view point, nesting may be difficult to supported in qemu guest agent, because some guest operating systems such as Windows cannot start new FSFreeze while some of filesystems are frozen.

Regards, Tomoki Sekiyama