Re: [Qemu-block] [Qemu-devel] [PATCHv2 for-2.4] block: Auto-generate node_names for each BDS entry

2015-06-23 Thread Markus Armbruster
Copying Paolo for advice on the more general problem.

Can you explain why we need this in 2.4?

Eric Blake ebl...@redhat.com writes:

 From: Jeff Cody jc...@redhat.com

 Currently, node_name is only filled in when done so explicitly by the
 user.  If no node_name is specified, then the node name field is not
 populated.

This is how we do user-specified IDs everywhere.

There are some exceptions for backward compatibility, where we make up
IDs when the user doesn't.  Those made-up IDs can clash with the user's,
so the user needs to either refrain from using IDs that could clash, or
take care to suppress them by specifying his own.  For the former, you
need to know all the made-up ID formats.  For the latter, you either
have to know exactly when you have to suppress a made-up ID, or you have
to specify IDs always, without fail.  That's libvirt's strategy.
Reliable as long as things don't acquire IDs later on, with made-up IDs
that can clash.  If we did that, then old libvirt won't suppress the
made-up IDs, because it has no idea it could specify one.

 If node_names are automatically generated when not specified, that means
 that all block job operations can be done by reference to the unique
 node_name field.  This eliminates ambiguity in resolving filenames
 (relative filenames, or file descriptors, symlinks, mounts, etc..) that
 qemu currently needs to deal with.

Let me rephrase the problem statement.

The user has to specify the object's ID to conveniently refer to it in
later commands.  If he neglects to specify one, referring to the object
in later commands becomes awkward or impossible, depending on the
command.

Example: command FROB lets you specify the node to frobnicate either by
parameter NODE-NAME or by parameter FILE.  Use of NODE-NAME is obvious
and unambiguous, but only possible when the user specified one.  Else,
you have to use the old and flawed FILE interface, which less obvious
and sometimes ambiguous: FILE is matched against file names, and file
names need not be unique.
(I lack the time today to find an actual command, so I made one up).

Example: command device_del lets you specified the device to be deleted
by its ID.  If the user neglected to specify one, you can't delete it.

My point is: the problem is more general than just block nodes.  Doesn't
mean we mustn't solve the special problem unless we solve the general
problem.  Does mean we should at least try to solve the general problem,
and if we fail, try our best to solve the special problem in a way that
fits into a general solution.

Have you tried to solve or at least discuss the general problem?

 If a node name is specified, then it will not be automatically
 generated for that BDS entry.

 If it is automatically generated, it will be prefaced with __qemu##,
 followed by 8 characters of a unique number, followed by 8 random
 ASCII characters in the range of 'A-Z'.  Some sample generated node-name
 strings:
 __qemu##IAIYNXXR
 __qemu##0002METXTRBQ
 __qemu##0001FMBORDWG

 The prefix is to aid in identifying it as a qemu-generated name, the
 numeric portion is to guarantee uniqueness in a given qemu session, and
 the random characters are to further avoid any accidental collisions
 with user-specified node-names.

I believe this is a good deal more complicated than need be.

The user's IDs (and this includes node names) have to satisfy
id_wellformed(): letters, digits, '-', '.', '_', starting with a letter.

Thus, a made-up ID starting with '_' cannot clash.  Instead of an
elaborately decorated (and hard to type) __qemu##0002METXTRBQ simple
_2 would do.

If you're less laconically minded than I am, pick something in between.
But please spare me strings of randome letters :)

 Reviewed-by: Eric Blake ebl...@redhat.com
 Signed-off-by: Jeff Cody jc...@redhat.com
 Message-Id: 
 9516d2684d419e1bfa2a95f66d2e9a70a4ff7eb7.1403723862.git.jc...@redhat.com
 [id_wellformed() rejects generated names, which means we can't collide]
 Signed-off-by: Eric Blake ebl...@redhat.com
 ---

 v2: revive this patch; very little needed changing

 I just posted a libvirt patch series that depends on this patch:
 https://www.redhat.com/archives/libvir-list/2015-June/msg0.html

 As the original was posted by Jeff (nearly a year ago!), it counts
 as a patch that was submitted prior to soft freeze :)

 [I will be on vacation the next 3 weeks, so I may be slow to respond
 on replies to this message]

  block.c | 22 +-
  1 file changed, 17 insertions(+), 5 deletions(-)

 diff --git a/block.c b/block.c
 index dd4f58d..2532b70 100644
 --- a/block.c
 +++ b/block.c
 @@ -765,16 +765,28 @@ static int bdrv_open_flags(BlockDriverState *bs, int 
 flags)
  return open_flags;
  }

 +#define GEN_NODE_NAME_PREFIX__qemu##
 +#define GEN_NODE_NAME_MAX_LEN   (sizeof(GEN_NODE_NAME_PREFIX) + 8 + 8)
  static void bdrv_assign_node_name(BlockDriverState *bs,
const char *node_name,

Re: [Qemu-block] [Qemu-devel] [PATCHv2 for-2.4] block: Auto-generate node_names for each BDS entry

2015-06-23 Thread Paolo Bonzini


On 23/06/2015 11:00, Markus Armbruster wrote:
 My point is: the problem is more general than just block nodes.  Doesn't
 mean we mustn't solve the special problem unless we solve the general
 problem.  Does mean we should at least try to solve the general problem,
 and if we fail, try our best to solve the special problem in a way that
 fits into a general solution.
 
 Have you tried to solve or at least discuss the general problem?

IIRC blockdevs are special because some of them (the backing files) are
opened without request from the user.

The same happens for board-created devices, but we can just say they
aren't unpluggable.

Paolo