qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Christian Borntraeger <borntraeger@de.ibm.com>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: kwolf@redhat.com, thuth@linux.vnet.ibm.com,
	Public KVM Mailing List <qemu-devel@nongnu.org>,
	Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>,
	armbru@redhat.com, mihajlov@linux.vnet.ibm.com,
	dahi@linux.vnet.ibm.com, stefanha@redhat.com,
	cornelia.huck@de.ibm.com, pbonzini@redhat.com
Subject: Re: [Qemu-devel] [PATCH v5 0/5] Geometry and blocksize detection for backing devices.
Date: Tue, 13 Jan 2015 09:32:22 +0100	[thread overview]
Message-ID: <54B4D816.6010507@de.ibm.com> (raw)
In-Reply-To: <20150102125747.GG10823@stefanha-thinkpad.redhat.com>

Am 02.01.2015 um 13:57 schrieb Stefan Hajnoczi:
> On Thu, Dec 18, 2014 at 04:59:50PM +0100, Christian Borntraeger wrote:
>> Are you ok with the patches? If yes, can you take care of these patches in the block tree?
> 
> This series looks close, I've left comments on the patches.

OK, so you would take care of the series in your tree if the next spin addresses your comments, right?
> 
> The series is fine for command-line QEMU users where probing makes the
> command-line more convenient, so we can merge it.  But the approach is
> fundamentally wrong for stacks where libvirt is in use.
> Libvirt is unaware of the guest geometry and block sizes that are probed
> in QEMU by this patch series.  This breaks non-shared storage migration
> and also means libvirt-based tools that manipulate drives on a guest may
> inadvertently change the guest-visible geometry and cause disk problems.
> 
> For example, what happens when you copy the disk image off a host DASD
> and onto NFS?  QEMU no longer probes the geometry and the disk geometry
> has changed.
> 
> The right place to tackle guest-visible geometry is in libvirt, not in
> QEMU, because it is guest state the needs to be captured in domain XML
> so that migration and tooling can preserve it when manipulating guests.
> 
> Stefan
> 

I agree that this is not perfect and has obvious holes, but  it works 
just fine with libvirt stacks that are typical on s390 (*). scsi disks
(emulated and real) are not affected and work just fine. DASD disks are
special anyway - please consider this as some kind of real HW pass-through
even when we use virtio-blk. We can assume that admins can provide
shared access between source and target. If you look at the real HW (LPAR
and z/VM) DASDs are always attached via fibre channel (FICON protocol)
(often with SAN switches) so shared access is quite common even over longer
distances.

And yes, using NFS or image file will break unless the user specifies the
geometry in libvirt. Setting these values is possible as of today in libvirts
XML. But what programmatic way should libvirt use to detect that information 
itself? On an image file libvirt doesnt know either. This would be somewhat
possible on an upper level like open stack and that upper level would then
need to talk with the DS8000 storage subsystems and the system z hardware but
even then it would fail when creating new DASD like images.  (This would boil
down to provide an interface like "create an image file that looks like as
DASD of type 3390/xx formatted with dasdfmt and the following parameters).


If we talk about cloud/openstack etc we do not have pass through devices anyway
and only image files. If somebody asks me, I would create all image files as 
SCSI images anyway, all the DASD stuff makes sense if you have DASD hardware 
(or if you really know what you are going to do).

What is your opinion on 
a) migrating disk properties like geometry
b) comparing the detected disk properties and reject migration if they
   dont match?
Both changes  should provide a safety net and could be added at a later
point in time.

I hope that clarifies some of the aspects and why I think that this patch series would be "good enough" for most cases. Makes sense?

Christian

PS: Proper DASD support might require a DASD emulation and/or passthrough
without virtio-blk. There are some very special operations (like low-level
format, raw-track access, reserve/release) which are pretty hard to 
virtualize with virtio-blk.

(*) Well, thats my guess,  as there are not many stacks on s390 yet in production 
as far as I know

  reply	other threads:[~2015-01-13  8:32 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-18 11:17 [Qemu-devel] [PATCH v5 0/5] Geometry and blocksize detection for backing devices Ekaterina Tumanova
2014-12-18 11:18 ` [Qemu-devel] [PATCH v5 1/5] block: add bdrv functions for geometry and blocksize Ekaterina Tumanova
2014-12-18 11:18 ` [Qemu-devel] [PATCH v5 2/5] raw-posix: Refactor logical block size detection Ekaterina Tumanova
2015-01-02 11:52   ` Stefan Hajnoczi
2015-01-13 10:03     ` Ekaterina Tumanova
2015-01-13 15:24       ` Stefan Hajnoczi
2014-12-18 11:18 ` [Qemu-devel] [PATCH v5 3/5] block: Add driver methods to probe blocksizes and geometry Ekaterina Tumanova
2014-12-18 12:43   ` Thomas Huth
2015-01-02 12:11   ` Stefan Hajnoczi
2014-12-18 11:18 ` [Qemu-devel] [PATCH v5 4/5] block-backend: Add wrappers for blocksizes and geometry probing Ekaterina Tumanova
2014-12-18 14:45   ` Thomas Huth
2015-01-02 12:34   ` Stefan Hajnoczi
2014-12-18 11:18 ` [Qemu-devel] [PATCH v5 5/5] BlockConf: Call backend functions to detect geometry and blocksizes Ekaterina Tumanova
2014-12-18 14:55   ` Thomas Huth
2015-01-02 12:46   ` Stefan Hajnoczi
2014-12-18 15:59 ` [Qemu-devel] [PATCH v5 0/5] Geometry and blocksize detection for backing devices Christian Borntraeger
2015-01-02 12:57   ` Stefan Hajnoczi
2015-01-13  8:32     ` Christian Borntraeger [this message]
2015-01-13 10:51       ` Markus Armbruster
2015-01-13 16:04         ` Stefan Hajnoczi
2015-01-13 17:27           ` Markus Armbruster
2015-01-13 19:07           ` Christian Borntraeger
2015-01-14 13:57             ` Stefan Hajnoczi
2015-01-02 11:30 ` Stefan Hajnoczi
2015-01-13  9:59   ` Ekaterina Tumanova

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=54B4D816.6010507@de.ibm.com \
    --to=borntraeger@de.ibm.com \
    --cc=armbru@redhat.com \
    --cc=cornelia.huck@de.ibm.com \
    --cc=dahi@linux.vnet.ibm.com \
    --cc=kwolf@redhat.com \
    --cc=mihajlov@linux.vnet.ibm.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    --cc=stefanha@redhat.com \
    --cc=thuth@linux.vnet.ibm.com \
    --cc=tumanova@linux.vnet.ibm.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 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).