On Thu, Feb 25, 2010 at 01:19:54PM +0100, Daniel Veillard wrote:
> On Wed, Feb 24, 2010 at 03:51:45PM -0500, Cole Robinson wrote:
> > Hi guys,
> >
> > Looking at the new FS pool build options and talking with Dave, I see that
> > calling PoolBuild on an FS pool now unconditionally calls mkfs. This is really
> > bad when mixed with virt-manager: previously, we assumed the FS build command
> > was always non destructive (at most it created a directory), so we called it
> > every time, and didn't even allow users to opt out, since there wasn't a use
> > case that called for it.
> >
> > This new formatting behavior really needs to be opt in, otherwise all
> > virt-manager versions creating an FS pool can destroy data.
> >
> > Just FYI, for disk pools (and certain LVM configurations) where this operation
> > has always been destructive, we default to build=off, and loudly warn the user
> > if they choose otherwise. We can do that with this new option as well, but the
> > previous behavior really needs to be reinstated IMO (and before the new release).
> >
> > I fully accept that this could be a bug in virt-manager's assumptions of the
> > build command, but even consider a virsh user: previously build just created a
> > directory, now it formats a partition, without any XML change.
>
> I was initially reluctant of changing the behaviour, and asked to use a
> flag to keep the original default semantic. I got convinced that noone
> could rely on it because the function was basically incomplete. But since
> virt-manager ships with an expectation on the previous behaviour, I
> revert my position, we need to add a _FORMAT = 4 flag for this call and
> only call mkfs if that flag is passed. Fix is trivial we should not
> push 0.7.7 without it,
I really don't want to add an extra flag, because it makes filesystem
pool a special case. The 'build' operation is intentionally destructive
by its very definition, and virt-mnager should never be expecting it to
be safe to call on specific pool types.
IMHO, we should do two things to address this
- Fix virt-manager to not call build all the time for any pool
type - it must only do it when expkicitly requested
- Make the 'build' operation check to see if the pool is
already constructed (eg LVM magic check for logical pools,
FAT partition check for disk ools & filesystem magic check
for the fs pool). Reject the build operation if any of these
show that the pool exists / is alread ybuilt
- Add a 'OVERWRITE' flag, to allow apps to forcably reformat,
regardless of current state
This will let us keep consistent semantics for all pool types, while still
protecting against broken apps like virt-manager which are blindly calling
build when they shouldn't.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list