Hi Daniel, On 4/23/14 5:55 , "Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote: >On Tue, Apr 22, 2014 at 06:22:18PM +0000, Tomoki Sekiyama wrote: >> Hi Daniel, >> thanks for your comment. >> >> On 4/22/14 11:39 , "Daniel P. Berrange" <berrange@xxxxxxxxxx> 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. > >Hmm, I wonder how practical such a conversion will be. > >eg libvirt has a block device "sda", but the guest OS may have added >a partition table (sda1, sda2, sda3, etc) and then put some of those >partitions into LVM (volgroup00) and then created logical volume >(vol1, vol2, etc). The guest agent can freeze individual filesystems >on each logical volume, so if the API is just taking libvirt block >device names, we can't express any way to freeze the filesystems the >guest has. Specifying libvirt disk alias name is coming from applications' requirement. For example, OpenStack cinder driver only knows provide libvirt device names. It is also nice if virDomainSnapshotCreateXML can specify 'disks' to be frozen when only a subset of the disks is specified in snapshot XML. I'm now prototyping qemu-guest-agent code to resolve filesystems from disk addresses, and it is working with virtio/SATA/IDE/SCSI drives on x86 Linux guests. It can also handle LVM logical volumes that lies on multiple partitions on multiple disks. https://github.com/tsekiyama/qemu/commit/6d26115e769a7fe6aba7be52d2180453ac a5fee5 This gathers disk device information from sysfs in the guest. On windows, I hope Virtual Disk Service can provide this kind of informations too. >So I think we have no choice but to actually have the API take a >list of guest "volumes" (eg mount points in Linux, or drive letters >in Windows). > >Ideally the guest agent would also provide a way to list all >currently known guest "volumes" so we could expose that in the >API too later. Possibly. If the volumes information from the API contains the dependent hardware addresses, libvirt clients might be able to map the volumes and libvirt disks from domain XML. In this case, specifying subset volumes in virDomainSnapshotCreateXML would be difficult. Maybe libvirt should provide the mapping function. Which way do you prefer? >> >> + * @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. > >Yep, I hadn't even considered that impl complexity. I agree we should >just drop the notion of nesting and say that we'll return >VIR_ERR_OPERATION_INVALID if someone attempts to freeze an already >frozen FS. OK, I will drop nesting. >Regards, >Daniel Regards, Tomoki Sekiyama -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list