0a122b
From 339b5cbcea1d459d2a0fc4d289e17fc71622be23 Mon Sep 17 00:00:00 2001
0a122b
From: Jeffrey Cody <jcody@redhat.com>
0a122b
Date: Tue, 4 Mar 2014 19:38:27 +0100
0a122b
Subject: [PATCH 6/6] block: Set block filename sizes to PATH_MAX instead of 1024
0a122b
MIME-Version: 1.0
0a122b
Content-Type: text/plain; charset=UTF-8
0a122b
Content-Transfer-Encoding: 8bit
0a122b
0a122b
RH-Author: Jeffrey Cody <jcody@redhat.com>
0a122b
Message-id: <cd1c834d5d15020c9521e95aa24d7db675dc12e2.1393961758.git.jcody@redhat.com>
0a122b
Patchwork-id: 58013
0a122b
O-Subject: [RHEL7 qemu-kvm PATCH] block: Set block filename sizes to PATH_MAX instead of 1024
0a122b
Bugzilla: 1072339
0a122b
RH-Acked-by: Eric Blake <eblake@redhat.com>
0a122b
RH-Acked-by: Amos Kong <akong@redhat.com>
0a122b
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
0a122b
0a122b
This is an interim fix for a regression introduced by:
0a122b
0a122b
    commit dc5a137125d6ac641c566f10e68bf6e1fe31bcb5
0a122b
    qemu-img: make "info" backing file output correct and easier to use
0a122b
0a122b
In that commit, bdrv_get_full_backing_filename() was introduced,
0a122b
which replaced calling path_combine() manually.  The difference
0a122b
is that rather than using the filename string as passed to
0a122b
bdrv_open(), it uses the filename string attached to the BDS.
0a122b
0a122b
Both the backing_file and filename strings in the BDS are limited to
0a122b
1024 characters.  The backing_file string built up in bdrv_open(),
0a122b
however, has a limit of PATH_MAX, which is 4096 under Linux.
0a122b
0a122b
This difference comes into play when using an image chain that has
0a122b
a medium-to-large number of images, all of which have relative-pathed
0a122b
backing file references to the parent directory.
0a122b
0a122b
For instance, consider the following backing chain:
0a122b
0a122b
tstA
0a122b
├── base.qcow2
0a122b
├── sn1.qcow2   (backing file ../tstA/base.qcow2)
0a122b
├── sn2.qcow2   (backing file ../tstA/sn1.qcow2)
0a122b
└── sn3.qcow2   (backing file ../tstA/sn2.qcow2)
0a122b
0a122b
The backing filename string is built up with the relative paths intact,
0a122b
like so:
0a122b
0a122b
    base.qcow2:  ../tstA../tstA../tstA/base.qcow2
0a122b
0a122b
The backing filename is then passed into the bdrv_open() call to open
0a122b
the backing file.
0a122b
0a122b
When using lv volume names, the snapshot and pathname ends up longer,
0a122b
and after ~23 snapshots we have hit or exceeded the 1024 byte limit
0a122b
for the BDS filename.
0a122b
0a122b
This fix is different then the approach for RHEL6.6/6.5.z, because in
0a122b
those it was trivial to modify bdrv_get_full_backing_filename().  In
0a122b
RHEL7, there are places that use bdrv_get_backing_filename(), which
0a122b
call bdrv_get_full_backing_filename(), yet do not have access to a
0a122b
filename string that does not originate from the BDS.  The simplest
0a122b
approach, that should yield identical results, is to set all of the
0a122b
filename and backing_file string sizes to PATH_MAX instead of 1024.
0a122b
0a122b
This is not a long-term solution, because a character limit of 4096
0a122b
bytes will be hit with additional images.  The proper long-term
0a122b
solution should happen upstream first, and consist of:
0a122b
0a122b
1) dynamically allocated filename strings in the BDS
0a122b
2) flattening redundant relative pathname strings
0a122b
0a122b
This is a bug that was reported in RHEL6, that also occurs in RHEL7.  To
0a122b
prevent a regression in RHEL7.0, this temporary solution will prevent
0a122b
the regression, while not eliminating the ultimate problem.
0a122b
0a122b
Since this is not the final solution, and the fix really is relevant
0a122b
just to undo a regression, the fix is downstream only.  It will be
0a122b
replaced by the final upstream fix, once complete.
0a122b
0a122b
BZ 1072339
0a122b
RHEL Brew: http://brewweb.devel.redhat.com/brew/taskinfo?taskID=7140291
0a122b
RHEV Brew: http://brewweb.devel.redhat.com/brew/taskinfo?taskID=7140320
0a122b
---
0a122b
 block/mirror.c            | 2 +-
0a122b
 block/qapi.c              | 2 +-
0a122b
 block/stream.c            | 2 +-
0a122b
 include/block/block_int.h | 6 +++---
0a122b
 4 files changed, 6 insertions(+), 6 deletions(-)
0a122b
0a122b
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
0a122b
---
0a122b
 block/mirror.c            |    2 +-
0a122b
 block/qapi.c              |    2 +-
0a122b
 block/stream.c            |    2 +-
0a122b
 include/block/block_int.h |    6 +++---
0a122b
 4 files changed, 6 insertions(+), 6 deletions(-)
0a122b
0a122b
diff --git a/block/mirror.c b/block/mirror.c
0a122b
index ba1428b..47e14cd 100644
0a122b
--- a/block/mirror.c
0a122b
+++ b/block/mirror.c
0a122b
@@ -297,7 +297,7 @@ static void coroutine_fn mirror_run(void *opaque)
0a122b
     int64_t sector_num, end, sectors_per_chunk, length;
0a122b
     uint64_t last_pause_ns;
0a122b
     BlockDriverInfo bdi;
0a122b
-    char backing_filename[1024];
0a122b
+    char backing_filename[PATH_MAX];
0a122b
     int ret = 0;
0a122b
     int n;
0a122b
 
0a122b
diff --git a/block/qapi.c b/block/qapi.c
0a122b
index 77e1719..2d4bdcd 100644
0a122b
--- a/block/qapi.c
0a122b
+++ b/block/qapi.c
0a122b
@@ -112,7 +112,7 @@ void bdrv_query_image_info(BlockDriverState *bs,
0a122b
 {
0a122b
     uint64_t total_sectors;
0a122b
     const char *backing_filename;
0a122b
-    char backing_filename2[1024];
0a122b
+    char backing_filename2[PATH_MAX];
0a122b
     BlockDriverInfo bdi;
0a122b
     int ret;
0a122b
     Error *err = NULL;
0a122b
diff --git a/block/stream.c b/block/stream.c
0a122b
index 3a7d8f3..2a6f533 100644
0a122b
--- a/block/stream.c
0a122b
+++ b/block/stream.c
0a122b
@@ -32,7 +32,7 @@ typedef struct StreamBlockJob {
0a122b
     RateLimit limit;
0a122b
     BlockDriverState *base;
0a122b
     BlockdevOnError on_error;
0a122b
-    char backing_file_id[1024];
0a122b
+    char backing_file_id[PATH_MAX];
0a122b
 } StreamBlockJob;
0a122b
 
0a122b
 static int coroutine_fn stream_populate(BlockDriverState *bs,
0a122b
diff --git a/include/block/block_int.h b/include/block/block_int.h
0a122b
index 2ec4bb2..53fc98c 100644
0a122b
--- a/include/block/block_int.h
0a122b
+++ b/include/block/block_int.h
0a122b
@@ -269,9 +269,9 @@ struct BlockDriverState {
0a122b
     const BlockDevOps *dev_ops;
0a122b
     void *dev_opaque;
0a122b
 
0a122b
-    char filename[1024];
0a122b
-    char backing_file[1024]; /* if non zero, the image is a diff of
0a122b
-                                this file image */
0a122b
+    char filename[PATH_MAX];
0a122b
+    char backing_file[PATH_MAX]; /* if non zero, the image is a diff of
0a122b
+                                    this file image */
0a122b
     char backing_format[16]; /* if non-zero and backing_file exists */
0a122b
     int is_temporary;
0a122b
 
0a122b
-- 
0a122b
1.7.1
0a122b