All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@suse.de>
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: "Li, Haicheng" <haicheng.li@intel.com>,
	"Xu, Dongxiao" <dongxiao.xu@intel.com>,
	xen-devel@lists.xensource.com
Subject: Re: VMX status report. Xen: #17630 & Xen0: #540 -- blocked
Date: Thu, 15 May 2008 15:53:32 +0200	[thread overview]
Message-ID: <482C405C.8000303@suse.de> (raw)
In-Reply-To: <18476.13699.632075.410501@mariner.uk.xensource.com>

Ian Jackson schrieb:
> Kevin Wolf writes ("Re: [Xen-devel] VMX status report. Xen: #17630 & Xen0: #540 -- blocked"):
>> Ian Jackson schrieb:
>>  > Could those of you having this problem please try the attached patch ?
>>  > I have tested this one much more thoroughly :-/.
>>
>> It works for me, I don't like it though. Maybe I don't understand 
>> correctly what the intent of the whole protocol stuff was. But if I do, 
>> I think you're destroying this with your patch. This is not necessarily 
>> bad as we don't use it anyway. But then it would be much cleaner to 
>> remove the functionality altogether.
> 
> I think you've misunderstood, but I could be wrong.  To me it seems
> that Qemu uses the terms `protocol' and `format' almost
> interchangeably - I don't think they refer to different things.

I certainly don't want to exclude that I've misunderstood. But if it was 
meant to select the image format, the code looks overcomplicated to me.

And now that upstream qemu has a -drive parameter with a format option, 
we could get rid of this filename parsing in both ioemu and upstream 
qemu then. In Xen we don't use it anyway.

> find_protocol, find_format and find_image_format all select from the
> same set of bdrv_xxx drivers: find_format take a name and returns the
> driver with that name; find_protocol takes a filename and looks for a
> `:' in it, and if so it assumes that the part before the `:' is the
> name and finds the driver with that name.  (Just for extra confusion
> the two namespaces are different and some drivers don't have a name
> that can be put in filenames, but some drivers have both names.)
> find_image_format reads the start of the file and passes it to drivers
> until one of them recognises it.

I think I understand quite well how it's processed in the code. ;-) And 
I also think that the code allows both of our understandings. If you 
wanted, I think you could add a "protocol" which in fact acts like a 
network protocol.

And the different namespaces still don't make too much sense to me if 
formats and protocols are really meant to be the same...

>> To be a bit more concrete, I think the following change is wrong (even 
>> if there is no user anyway):
>>
>> -    /* no need to test disk image formats for vvfat */
>> -    if (drv == &bdrv_vvfat)
>> +    /* no need to test disk image format if the filename told us */
>> +    if (drv != NULL)
>>           return drv;
>>
>> find_protocol doesn't tell you the image format of a file, it tells you 
>> a protocol through which you should obtain the image. And the image you 
>> get could be a qcow image then.
> 
> These `protocols' aren't network protocols, they're disk image
> formats.  The only ones which aren't a disk image format are vvfat and
> (in ioemu) vbd.
> 
> vvfat is a crazy thing which you can't currently sanely layer anything
> on top of (even though you might want to) and for which layering
> a block driver underneath makes no sense.

I agree that vvfat isn't really a protocol as I understand them. It's 
only another block driver and it's using the protocol thing only because 
there was no format option qemu until recently. Maybe you're right and 
the "protocol" was only introduced as a hack to allow overriding the 
image format.

Kevin

  reply	other threads:[~2008-05-15 13:53 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-14  9:50 VMX status report. Xen: #17630 & Xen0: #540 -- blocked Li, Haicheng
2008-05-14 10:44 ` Ian Jackson
2008-05-15  7:40   ` Li, Haicheng
2008-05-15  8:30     ` Xu, Dongxiao
2008-05-15  9:00       ` Kevin Wolf
2008-05-15  9:31         ` Xu, Dongxiao
2008-05-15  9:50           ` Kevin Wolf
2008-05-15 11:07             ` Ian Jackson
2008-05-15 11:54               ` Kevin Wolf
2008-05-15 13:07                 ` Ian Jackson
2008-05-15 13:53                   ` Kevin Wolf [this message]
2008-05-16  1:38               ` Xu, Dongxiao
2008-05-16  9:14                 ` Ian Jackson

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=482C405C.8000303@suse.de \
    --to=kwolf@suse.de \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=dongxiao.xu@intel.com \
    --cc=haicheng.li@intel.com \
    --cc=xen-devel@lists.xensource.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.