|
|
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:
|
|
|
0a122b |
RHEV Brew: http:
|
|
|
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
|
|
|
0a122b |
index ba1428b..47e14cd 100644
|
|
|
0a122b |
|
|
|
0a122b |
|
|
|
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
|
|
|
0a122b |
index 77e1719..2d4bdcd 100644
|
|
|
0a122b |
|
|
|
0a122b |
|
|
|
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
|
|
|
0a122b |
index 3a7d8f3..2a6f533 100644
|
|
|
0a122b |
|
|
|
0a122b |
|
|
|
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
|
|
|
0a122b |
index 2ec4bb2..53fc98c 100644
|
|
|
0a122b |
|
|
|
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];
|
|
|
0a122b |
- this file image */
|
|
|
0a122b |
+ char filename[PATH_MAX];
|
|
|
0a122b |
+ char backing_file[PATH_MAX];
|
|
|
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 |
|