All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Lalancette <clalance@redhat.com>
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>
Subject: Re: [PATCH 0/4]: Expand xvd to support > 16 devices
Date: Wed, 25 Jun 2008 11:45:15 +0200	[thread overview]
Message-ID: <486213AB.7020401@redhat.com> (raw)
In-Reply-To: <18527.63569.430144.628906@mariner.uk.xensource.com>

(sorry for the delay in responding)

Ian Jackson wrote:
> When the guest is enumerating the devices, it should be sure to
> check that the block device number integer matches one of the expected
> forms, as I wrote in my message.  If the number does not, then that
> vbd should be ignored with a warning message.

OK, yes, I see, that makes sense.  I'll make the appropriate change in xlvbd_add().

> 
> This applies also to the partition numbers which you are currently
> limiting to 15.  That's fine but you should put in a check so that
> out-of-range partition numbers are ignored rather than causing
> unexpected behaviours.  (I'll admit that I haven't analysed your code
> in detail to determine exactly what the behaviour would be ...)

I'm not sure that this one is a problem (although I could be wrong).  During
xlvbd_add() time, we end up doing an alloc_disk() with the number of minors that
we can use.  So I don't think that the rest of the system will allow us to go
beyond that value; empirical evidence seems to support this, as attaching a disk
with 16 partitions to /dev/xvdb only shows the first 15 partitions.

Incidentally, the comment I made in my initial posting about expanding the
partitions is wrong, looking back at the code.  I *did* expand the number of
entries that blkfront will pick up (i.e. increased nr_minors when doing the
alloc_disk()), but I did not change the tools side to accept partitions > 15.
Again, something that can easily be done in the future.

> Also I think you should include an API changelog entry.

Do you mean on the Xen Wiki?  I did find a page about API changes, so if/when
these patches go in, I'm happy to add an entry there.

Thanks for looking,
Chris Lalancette

  reply	other threads:[~2008-06-25  9:45 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-23 18:26 [PATCH 1/4]: Fix blkfront to accept expanded devices Chris Lalancette
2008-06-23 18:26 ` [PATCH 0/4]: Expand xvd to support > 16 devices Chris Lalancette
2008-06-23 19:24   ` Ian Jackson
2008-06-25  9:45     ` Chris Lalancette [this message]
2008-06-25 11:19       ` Keir Fraser

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=486213AB.7020401@redhat.com \
    --to=clalance@redhat.com \
    --cc=Ian.Jackson@eu.citrix.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.