qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Anthony Liguori <anthony@codemonkey.ws>
To: Kevin Wolf <kwolf@redhat.com>
Cc: Christoph Hellwig <chellwig@redhat.com>,
	qemu-devel@nongnu.org, Markus Armbruster <armbru@redhat.com>,
	Luiz Capitulino <lcapitulino@redhat.com>,
	Avi Kivity <avi@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [Qemu-devel] Re: RFC v2: blockdev_add & friends, brief rationale, QMP docs
Date: Thu, 17 Jun 2010 08:01:42 -0500	[thread overview]
Message-ID: <4C1A1CB6.2050705@codemonkey.ws> (raw)
In-Reply-To: <4C19DABC.8090609@redhat.com>

On 06/17/2010 03:20 AM, Kevin Wolf wrote:
> Am 16.06.2010 20:07, schrieb Anthony Liguori:
>    
>>>    But it requires that
>>> everything that -blockdev provides is accessible with -drive, too (or
>>> that we're okay with users hating us).
>>>
>>>        
>> I'm happy for -drive to die.  I think we should support -hda and
>> -blockdev.
>>      
> -hda is not sufficient for most users. It doesn't provide any options.
> It doesn't even support virtio. If -drive is going to die (and we seem
> to agree all on that), then -blockdev needs to be usable for users (and
> it's only you who contradicts so far).
>    

I've always thought we should have a -vda argument and an -sda argument 
specifically for specifying virtio and scsi disks.

>> -blockdev should be optimized for config files, not single
>> argument input.  IOW:
>>
>> [blockdev "blk2"]
>>    format = "raw"
>>    file = "/path/to/base.img"
>>    cache = "writeback"
>>
>> [blockdev "blk1"]
>>     format = "qcow2"
>>     file = "/path/to/leaf.img"
>>     cache="off"
>>     backing_dev = "blk2"
>>
>> [device "disk1"]
>>     driver = "ide-drive"
>>     blockdev = "blk1"
>>     bus = "0"
>>     unit = "0"
>>      
> You don't specify the backing file of an image on the command line (or
> in the configuration file).

But we really ought to allow it.  Backing files are implemented as part 
of the core block layer, not the actual block formats.  Today the block 
layer queries the block format for the name of the backing file but gets 
no additional options from the block format.  File isn't necessarily 
enough information to successfully open the backing device so why treat 
it specially?

I think we should keep the current ability to query the block format for 
a backing file name but we should also support hooking up the backing 
device without querying the block format at all.  It makes the model 
much more elegant IMHO because then we're just creating block devices 
and hooking them up.  All block devices are created equal more or less.

>   It's saved as part of the image. It's more
> like this (for a simple raw image file):
>
> [blockdev-protocol "proto1"]
>     protocol = "file"
>     file = "/path/to/image.img"
>
> [blockdev "blk1"]
>     format = "raw"
>     cache="off"
>     protocol = "proto1"
>
> [device "disk1"]
>     driver = "ide-drive"
>     blockdev = "blk1"
>     bus = "0"
>     unit = "0"
>
> (This would be Markus' option 3, I think)
>    

I don't understand why we need two layers of abstraction here.  Why not 
just:

[blockdev "proto1"]
   protocol = "file"
   cache = "off"
   file = "/path/to/image.img"

Why does the cache option belong with raw and not with file and why 
can't we just use file directly?As Christoph mentions, we really don't 
have stacked protocols and I'm

>> not sure they make sense.
>>      
> Right, if we go for Christoph's suggestion, we don't need stacked
> protocols. We'll have stacked formats instead. I'm not sure if you like
> this any better. ;-)
>
> We do have stacking today. -hda blkdebug:test.cfg:foo.qcow2 is qcow2 on
> blkdebug on file. We need to be able to represent this.
>    

I think we need to do stacking in a device specific way.  When you look 
at something like vmdk, it should actually support multiple leafs since 
the format does support such a thing.  So what I'd suggest is:

[blockdev "part1"]
   format = "raw"
   file = "image000.vmdk"

[blockdev "part2"]
   format = "raw"
   file = "image001.vmdk"

[blockdev "image"]
   format = "vmdk"
   section0 = "part1"
   section1 = "part2"

Note, we'll need to support this sort of model in order to support a 
disk that creates an automatic partition table (which would be a pretty 
useful feature).  For blkdebug, it would look like:

[blockdev "disk"]
   format = "qcow2"
   file = "foo.qcow2"

[blockdev "debug"]
   format = "blkdebug"
   blockdev = "disk"

>> I think raw doesn't make very much sense then.  What's the point of it
>> if it's just a thin wrapper around a protocol?
>>      
> That it can be wrapped around any protocol. It's just about separating
> code for handling the content of an image and code for accessing the image.
>
> Ever tried something like "qemu-img create -f raw /dev/something 10G"?
> You need the host_device protocol there, not the file protocol. When we
> had raw == file this completely failed. And it's definitely reasonable
> to expect that it works because the image format _is_ raw, it's just not
> saved in a file.
>    

No, I don't actually thing it's reasonable.  There's nothing meaningful 
that command can do.  Also, I've never understand creating qcow2 images 
on a physical device.  qcow2 needs to grow dynamically and physical 
devices can't.

I understand that we need to support the later use case but I don't 
think creating this layer of user-visible abstraction is the right thing 
to do.  This is an obscure use case and it shouldn't be the model that 
we force upon our users.

> Or the famous qcow2 images on block devices. Why did qemu guess the
> format correctly when qcow2 was saved in a file, but not on a host
> device? This was just inconsistent.
>
> I've had more than one bug report about things like this which are
> magically fixed when you do the layering right.
>    

Beyond qcow2 on physical devices, when does this issue actually come up?

Regards,

Anthony Liguori

> Kevin
>    

  reply	other threads:[~2010-06-17 13:01 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-10 17:45 [Qemu-devel] RFC v2: blockdev_add & friends, brief rationale, QMP docs Markus Armbruster
2010-06-15  9:04 ` Avi Kivity
2010-06-15 12:23   ` Markus Armbruster
2010-06-15 12:43     ` Avi Kivity
2010-06-15 13:27       ` Markus Armbruster
2010-06-15 13:40         ` Avi Kivity
2010-06-15 14:54           ` Markus Armbruster
2010-06-16  9:50             ` Avi Kivity
2010-06-16 11:02               ` Markus Armbruster
2010-06-16 11:06                 ` Avi Kivity
2010-06-15 13:44 ` [Qemu-devel] " Avi Kivity
2010-06-15 14:39   ` Gerd Hoffmann
2010-06-16 11:20   ` Kevin Wolf
2010-06-16 12:41     ` Markus Armbruster
2010-06-16 13:41       ` Anthony Liguori
2010-06-16 13:57         ` Kevin Wolf
2010-06-16 14:24           ` Christoph Hellwig
2010-06-16 14:47             ` Markus Armbruster
2010-06-16 18:07           ` Anthony Liguori
2010-06-17  8:20             ` Kevin Wolf
2010-06-17 13:01               ` Anthony Liguori [this message]
2010-06-17 14:15                 ` Kevin Wolf
2010-06-18  8:20                   ` Markus Armbruster
2010-06-18  9:36                     ` Kevin Wolf
2010-06-18  7:06                 ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4C1A1CB6.2050705@codemonkey.ws \
    --to=anthony@codemonkey.ws \
    --cc=armbru@redhat.com \
    --cc=avi@redhat.com \
    --cc=chellwig@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).