Re: [Qemu-devel] [PATCH] Fix -snapshot deleting CDROM images

[Markus Armbruster]

Blue Swirl <address@hidden> writes:

> Command line flag '-snapshot' was setting the drive flag 'snapshot'
> for all drives. Therefore also CDROM devices were incorrectly marked
> with BDRV_O_SNAPSHOT. Thus the backing images were accidentally deleted
> at bdrv_open time, for example when changing the image with monitor
> 'change' command.
>
> Fix by adding a separate 'global_snapshot' drive flag for use when the
> command line flag '-snapshot' is used. Also add some extra checks
> and suppress a kraxelian notation.

This patch doesn't feel right to me.

The bug you observed is that snapshot=on does something stupid for a
certain kind of drive: bdrv_open_common() removes a "temporary" file
that isn't temporary.  That bug needs fixing.  Your patch does not fix
it.

Instead, it attempts to avoid the bug: snapshot=on now fails with
media=cdrom, and the new -drive option global_snapshot=on gets silently
ignored with media=cdrom.

Why is media=cdrom the only case where the bug can bite?

Why not fix bdrv_open_common()?

[...]