At Thu, 13 May 2010 04:46:46 +0900, MORITA Kazutaka wrote: > > On 2010/05/12 20:38, Kevin Wolf wrote: > > I'll have a closer look at your code later, but one thing I noticed is > > that the new block driver is something in between a protocol and a > > format driver (just like vvfat, which should stop doing so, too). I > > think it ought to be a real protocol with the raw format driver on top > > (or any other format - I don't see a reason why this should be > > restricted to raw). > > > > The one thing that is unusual about it as a protocol driver is that it > > supports snapshots. However, while it is the first one, supporting > > snapshots in protocols is a thing that could be generally useful to > > support (for example thinking of a LVM protocol, which was discussed in > > the past). > > > > I agreed. I'll modify the sheepdog driver patch as a protocol driver one, > and remove unnecessary format check from my patch. > > > So in block.c we could check if the format driver supports snapshots, > > and if it doesn't we try again with the underlying protocol. Not sure > > yet what we would do when both format and protocol do support snapshots > > (qcow2 on sheepdog/LVM/...), but that's a detail. > > > To support snapshot in a protocol, I'd like to call the hander of the protocol driver in the following functions in block.c: bdrv_snapshot_create bdrv_snapshot_goto bdrv_snapshot_delete bdrv_snapshot_list bdrv_save_vmstate bdrv_load_vmstate Is it okay? In the case both format and protocol drivers support snapshots, I think it is better to call the format driver handler. Because qcow2 is well known as a snapshot support format, so when users use qcow2, they expect to get snapshot with qcow2. There is another problem to make the sheepdog driver be a protocol; how to deal with protocol specific create_options? For example, sheepdog supports cloning images as a format driver: $ qemu-img create -f sheepdog dst -b sheepdog:src But if the sheepdog driver is a protocol, error will occur. $ qemu-img create sheepdog:dst -b sheepdog:src Unknown option 'backing_file' qemu-img: Backing file not supported for file format 'raw' It is because the raw format doesn't support a backing_file option. To support the protocol specific create_options, if the format driver cannot parse some of the arguments, the protocol driver need to parse them. If my suggestions are okay, I'd like to prepare the patches. Regards, Kazutaka -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html