[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 04/14] qemu-nbd: Simplify --partition handling
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 04/14] qemu-nbd: Simplify --partition handling |
Date: |
Fri, 30 Nov 2018 16:41:47 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.0 |
On 11/30/18 4:26 PM, Richard W.M. Jones wrote:
On Fri, Nov 30, 2018 at 04:03:33PM -0600, Eric Blake wrote:
Our open-coding of strtol handling forgot to handle overflow
conditions. What's more, since we insiste on a user-supplied
"insist"
(Ever wonder if I stick in a typo on purpose, just to see who reviews
closely?)
partition to be non-zero, we can use 0 rather than -1 for our
initial value to distinguish when a partition is not being
served, for slightly more optimal code.
Signed-off-by: Eric Blake <address@hidden>
---
qemu-nbd.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
+ if (qemu_strtoi(optarg, NULL, 0, &partition) < 0 ||
+ partition < 1 || partition > 8) {
+ error_report("Invalid partition %s", optarg);
Pffft only supporting a mere 8 partitions :-? I raised the limits in
nbdkit recently so it can handle an infinite number of partitions :-)
nbdkit also handles GPT partitions. qemu-nbd is still stuck on MBR:
static int find_partition(BlockBackend *blk, int partition,
off_t *offset, off_t *size)
{
struct partition_record mbr[4];
uint8_t data[MBR_SIZE];
and if I read git blame correctly, the partition code in qemu-nbd is
mostly untouched since Fabrice committed Anthony's initial
implementation in 2008.
Also, reading the code, I see that qemu-nbd allows partitions 5-8
because it reads through the extended partition descriptor in 4 to
expose the logical partitions within; which nbdkit can't do yet :)
Anyway, it's a problem with the existing code, so doesn't affect the
review.
Indeed, and with nbdkit around, I'm wondering if I should deprecate
'qemu-nbd --partition' and just point people to nbdkit instead
(provided, of course, that nbdkit learns logical partitions).
Even if you have a qcow2 image with MBR partitions, and where we don't
want to write a qcow2 plugin in nbdkit, you can still always rewrite:
client -> qemu-nbd --partition=1
into
client -> nbdkit --filter=partition nbd partition=1 -> qemu-nbd
(although I don't know what the performance penalty would be)
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
- [Qemu-devel] [PATCH for-4.0 00/14] nbd: add qemu-nbd --list, Eric Blake, 2018/11/30
- [Qemu-devel] [PATCH 04/14] qemu-nbd: Simplify --partition handling, Eric Blake, 2018/11/30
- [Qemu-devel] [PATCH 05/14] nbd/client: Drop pointless buf variable, Eric Blake, 2018/11/30
- [Qemu-devel] [PATCH 01/14] qemu-nbd: Use program name in error messages, Eric Blake, 2018/11/30
- [Qemu-devel] [PATCH 06/14] nbd/client: Move export name into NBDExportInfo, Eric Blake, 2018/11/30
- [Qemu-devel] [PATCH 02/14] nbd/client: More consistent error messages, Eric Blake, 2018/11/30
- [Qemu-devel] [PATCH 03/14] qemu-nbd: Fail earlier for -c/-d on non-linux, Eric Blake, 2018/11/30