[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V6 1/3] Implement sync modes for drive-backup.
From: |
Ian Main |
Subject: |
Re: [Qemu-devel] [PATCH V6 1/3] Implement sync modes for drive-backup. |
Date: |
Wed, 24 Jul 2013 15:01:24 -0700 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Wed, Jul 24, 2013 at 02:32:53PM -0600, Eric Blake wrote:
> On 07/24/2013 04:55 AM, Kevin Wolf wrote:
>
> > Unconditionally overriding format for NEW_IMAGE_MODE_EXISTING is
> > definitely wrong. It's the user's choice which COW format to use for the
> > backup image. There's no reason why it has to be the same format as the
> > image that is being backed up.
> >
> > Before, bs->drv->format_name was a default for the case where a new
> > image had to be created and no format was given; and the format of
> > existing images could be probed. This is still what makes most sense to
> > me. What's even the goal with this change?
Actually I think that code is wrong. If we are using
NEW_IMAGE_MODE_EXISTING then format doesn't get used. We just end up
using bdrv_open() below to open the existing image. Format should not
be specified for an existing image.
> Furthermore, I'm proposing that for 1.6, we should make the format
> argument mandatory for drive-backup. We made it optional for
> drive-mirror, to allow for probing, but there have been CVEs in the past
> due to probing of a raw file gone wrong. We can always relax a
> mandatory argument into an optional one in 1.7, if we decide that
> probing can be done safely, but we can never turn an optional argument
> into a mandatory one once the initial release bakes in the option. It
> would make the code a lot simpler to just have a mandatory format
> argument, instead of having to bake in and document hueristics on which
> format is picked when the caller doesn't provide one.
So I made format mandatory in the last patch but only for
NEW_IMAGE_MODE_ABSOLUTE_PATHS. It actually doesn't make sense to
specify the format of an existing image so I left it optional as an
argument, but it will throw an error if it's not specified for the case
where we create a new image.
That make sense?
Ian
[Qemu-devel] [PATCH V6 2/3] Add tests for sync modes 'TOP' and 'NONE', Ian Main, 2013/07/22
[Qemu-devel] [PATCH V6 3/3] Add backing drive while performing backup., Ian Main, 2013/07/22
Re: [Qemu-devel] [PATCH V6 0/3] Implement sync modes for drive-backup., Stefan Hajnoczi, 2013/07/23