Laurent Vivier (4):

block/ssh: Convert from DPRINTF() macro to trace events

block/curl: Convert from DPRINTF() macro to trace events

block/file-posix: Convert from DPRINTF() macro to trace events

block/sheepdog: Convert from DPRINTF() macro to trace events

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Render block nodes graph with help of graphviz. This new function is

for debugging, so there is no sense to put it into qemu.py as a method

of QEMUMachine. Let's instead put it separately.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Acked-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-id: 20181221170909.25584-3-vsementsov@virtuozzo.com

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

scripts/render_block_graph.py | 120 ++++++++++++++++++++++++++++++++++

1 file changed, 120 insertions(+)

create mode 100755 scripts/render_block_graph.py

diff --git a/scripts/render_block_graph.py b/scripts/render_block_graph.py

new file mode 100755

index XXXXXXX..XXXXXXX

--- /dev/null

+++ b/scripts/render_block_graph.py

@@ -XXX,XX +XXX,XX @@

+#!/usr/bin/env python

+#

+# Render Qemu Block Graph

+#

+# Copyright (c) 2018 Virtuozzo International GmbH. All rights reserved.

+#

+# This program is free software; you can redistribute it and/or modify

+# it under the terms of the GNU General Public License as published by

+# the Free Software Foundation; either version 2 of the License, or

+# (at your option) any later version.

+#

+# This program is distributed in the hope that it will be useful,

+# but WITHOUT ANY WARRANTY; without even the implied warranty of

+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the

+# GNU General Public License for more details.

+#

+# You should have received a copy of the GNU General Public License

+# along with this program. If not, see <http://www.gnu.org/licenses/>.

+#

+

+import os

+import sys

+import subprocess

+import json

+from graphviz import Digraph

+from qemu import MonitorResponseError

+

+

+def perm(arr):

+ s = 'w' if 'write' in arr else '_'

+ s += 'r' if 'consistent-read' in arr else '_'

+ s += 'u' if 'write-unchanged' in arr else '_'

+ s += 'g' if 'graph-mod' in arr else '_'

+ s += 's' if 'resize' in arr else '_'

+ return s

+

+

+def render_block_graph(qmp, filename, format='png'):

+ '''

+ Render graph in text (dot) representation into "@filename" and

+ representation in @format into "@filename.@format"

+ '''

+

+ bds_nodes = qmp.command('query-named-block-nodes')

+ bds_nodes = {n['node-name']: n for n in bds_nodes}

+

+ job_nodes = qmp.command('query-block-jobs')

+ job_nodes = {n['device']: n for n in job_nodes}

+

+ block_graph = qmp.command('x-debug-query-block-graph')

+

+ graph = Digraph(comment='Block Nodes Graph')

+ graph.format = format

+ graph.node('permission symbols:\l'

+ ' w - Write\l'

+ ' r - consistent-Read\l'

+ ' u - write - Unchanged\l'

+ ' g - Graph-mod\l'

+ ' s - reSize\l'

+ 'edge label scheme:\l'

+ ' <child type>\l'

+ ' <perm>\l'

+ ' <shared_perm>\l', shape='none')

+

+ for n in block_graph['nodes']:

+ if n['type'] == 'block-driver':

+ info = bds_nodes[n['name']]

+ label = n['name'] + ' [' + info['drv'] + ']'

+ if info['drv'] == 'file':

+ label += '\n' + os.path.basename(info['file'])

+ shape = 'ellipse'

+ elif n['type'] == 'block-job':

+ info = job_nodes[n['name']]

+ label = info['type'] + ' job (' + n['name'] + ')'

+ shape = 'box'

+ else:

+ assert n['type'] == 'block-backend'

+ label = n['name'] if n['name'] else 'unnamed blk'

+ shape = 'box'

+

+ graph.node(str(n['id']), label, shape=shape)

+

+ for e in block_graph['edges']:

+ label = '%s\l%s\l%s\l' % (e['name'], perm(e['perm']),

+ perm(e['shared-perm']))

+ graph.edge(str(e['parent']), str(e['child']), label=label)

+

+ graph.render(filename)

+

+

+class LibvirtGuest():

+ def __init__(self, name):

+ self.name = name

+

+ def command(self, cmd):

+ # only supports qmp commands without parameters

+ m = {'execute': cmd}

+ ar = ['virsh', 'qemu-monitor-command', self.name, json.dumps(m)]

+

+ reply = json.loads(subprocess.check_output(ar))

+

+ if 'error' in reply:

+ raise MonitorResponseError(reply)

+

+ return reply['return']

+

+

+if __name__ == '__main__':

+ obj = sys.argv[1]

+ out = sys.argv[2]

+

+ if os.path.exists(obj):

+ # assume unix socket

+ qmp = QEMUMonitorProtocol(obj)

+ qmp.connect()

+ else:

+ # assume libvirt guest name

+ qmp = LibvirtGuest(obj)

+

+ render_block_graph(qmp, out)

--

2.20.1

From: Laurent Vivier <lvivier@redhat.com>

Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: Richard W.M. Jones <rjones@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Message-id: 20181213162727.17438-2-lvivier@redhat.com

[mreitz: Fixed type of ssh_{read,write}_return's parameter to be ssize_t

instead of size_t]

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

block/ssh.c | 46 +++++++++++++++++-----------------------------

block/trace-events | 17 +++++++++++++++++

2 files changed, 34 insertions(+), 29 deletions(-)

diff --git a/block/ssh.c b/block/ssh.c

index XXXXXXX..XXXXXXX 100644

--- a/block/ssh.c

+++ b/block/ssh.c

@@ -XXX,XX +XXX,XX @@

#include "qapi/qmp/qstring.h"

#include "qapi/qobject-input-visitor.h"

#include "qapi/qobject-output-visitor.h"

+#include "trace.h"

-/* DEBUG_SSH=1 enables the DPRINTF (debugging printf) statements in

- * this block driver code.

- *

+/*

* TRACE_LIBSSH2=<bitmask> enables tracing in libssh2 itself. Note

* that this requires that libssh2 was specially compiled with the

* `./configure --enable-debug' option, so most likely you will have

* to compile it yourself. The meaning of <bitmask> is described

* here: http://www.libssh2.org/libssh2_trace.html

*/

-#define DEBUG_SSH 0

#define TRACE_LIBSSH2 0 /* or try: LIBSSH2_TRACE_SFTP */

-#define DPRINTF(fmt, ...) \

- do { \

- if (DEBUG_SSH) { \

- fprintf(stderr, "ssh: %-15s " fmt "\n", \

- __func__, ##__VA_ARGS__); \

- } \

- } while (0)

-

typedef struct BDRVSSHState {

/* Coroutine. */

CoMutex lock;

@@ -XXX,XX +XXX,XX @@ static int check_host_key_knownhosts(BDRVSSHState *s,

switch (r) {

case LIBSSH2_KNOWNHOST_CHECK_MATCH:

/* OK */

- DPRINTF("host key OK: %s", found->key);

+ trace_ssh_check_host_key_knownhosts(found->key);

break;

case LIBSSH2_KNOWNHOST_CHECK_MISMATCH:

ret = -EINVAL;

@@ -XXX,XX +XXX,XX @@ static int connect_to_ssh(BDRVSSHState *s, BlockdevOptionsSsh *opts,

}

/* Open the remote file. */

- DPRINTF("opening file %s flags=0x%x creat_mode=0%o",

- opts->path, ssh_flags, creat_mode);

+ trace_ssh_connect_to_ssh(opts->path, ssh_flags, creat_mode);

s->sftp_handle = libssh2_sftp_open(s->sftp, opts->path, ssh_flags,

creat_mode);

if (!s->sftp_handle) {

@@ -XXX,XX +XXX,XX @@ static int coroutine_fn ssh_co_create_opts(const char *filename, QemuOpts *opts,

/* Get desired file size. */

ssh_opts->size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),

BDRV_SECTOR_SIZE);

- DPRINTF("total_size=%" PRIi64, ssh_opts->size);

+ trace_ssh_co_create_opts(ssh_opts->size);

uri_options = qdict_new();

ret = parse_uri(filename, uri_options, errp);

@@ -XXX,XX +XXX,XX @@ static void restart_coroutine(void *opaque)

BDRVSSHState *s = bs->opaque;

AioContext *ctx = bdrv_get_aio_context(bs);

- DPRINTF("co=%p", restart->co);

+ trace_ssh_restart_coroutine(restart->co);

aio_set_fd_handler(ctx, s->sock, false, NULL, NULL, NULL, NULL);

aio_co_wake(restart->co);

@@ -XXX,XX +XXX,XX @@ static coroutine_fn void co_yield(BDRVSSHState *s, BlockDriverState *bs)

wr_handler = restart_coroutine;

}

- DPRINTF("s->sock=%d rd_handler=%p wr_handler=%p", s->sock,

- rd_handler, wr_handler);

+ trace_ssh_co_yield(s->sock, rd_handler, wr_handler);

aio_set_fd_handler(bdrv_get_aio_context(bs), s->sock,

false, rd_handler, wr_handler, NULL, &restart);

qemu_coroutine_yield();

- DPRINTF("s->sock=%d - back", s->sock);

+ trace_ssh_co_yield_back(s->sock);

}

/* SFTP has a function `libssh2_sftp_seek64' which seeks to a position

@@ -XXX,XX +XXX,XX @@ static void ssh_seek(BDRVSSHState *s, int64_t offset, int flags)

bool force = (flags & SSH_SEEK_FORCE) != 0;

if (force || op_read != s->offset_op_read || offset != s->offset) {

- DPRINTF("seeking to offset=%" PRIi64, offset);

+ trace_ssh_seek(offset);

libssh2_sftp_seek64(s->sftp_handle, offset);

s->offset = offset;

s->offset_op_read = op_read;

@@ -XXX,XX +XXX,XX @@ static coroutine_fn int ssh_read(BDRVSSHState *s, BlockDriverState *bs,

char *buf, *end_of_vec;

struct iovec *i;

- DPRINTF("offset=%" PRIi64 " size=%zu", offset, size);

+ trace_ssh_read(offset, size);

ssh_seek(s, offset, SSH_SEEK_READ);

@@ -XXX,XX +XXX,XX @@ static coroutine_fn int ssh_read(BDRVSSHState *s, BlockDriverState *bs,

*/

for (got = 0; got < size; ) {

again:

- DPRINTF("sftp_read buf=%p size=%zu", buf, end_of_vec - buf);

+ trace_ssh_read_buf(buf, end_of_vec - buf);

r = libssh2_sftp_read(s->sftp_handle, buf, end_of_vec - buf);

- DPRINTF("sftp_read returned %zd", r);

+ trace_ssh_read_return(r);

if (r == LIBSSH2_ERROR_EAGAIN || r == LIBSSH2_ERROR_TIMEOUT) {

co_yield(s, bs);

@@ -XXX,XX +XXX,XX @@ static int ssh_write(BDRVSSHState *s, BlockDriverState *bs,

char *buf, *end_of_vec;

struct iovec *i;

- DPRINTF("offset=%" PRIi64 " size=%zu", offset, size);

+ trace_ssh_write(offset, size);

ssh_seek(s, offset, SSH_SEEK_WRITE);

@@ -XXX,XX +XXX,XX @@ static int ssh_write(BDRVSSHState *s, BlockDriverState *bs,

for (written = 0; written < size; ) {

again:

- DPRINTF("sftp_write buf=%p size=%zu", buf, end_of_vec - buf);

+ trace_ssh_write_buf(buf, end_of_vec - buf);

r = libssh2_sftp_write(s->sftp_handle, buf, end_of_vec - buf);

- DPRINTF("sftp_write returned %zd", r);

+ trace_ssh_write_return(r);

if (r == LIBSSH2_ERROR_EAGAIN || r == LIBSSH2_ERROR_TIMEOUT) {

co_yield(s, bs);

@@ -XXX,XX +XXX,XX @@ static coroutine_fn int ssh_flush(BDRVSSHState *s, BlockDriverState *bs)

{

int r;

- DPRINTF("fsync");

+ trace_ssh_flush();

again:

r = libssh2_sftp_fsync(s->sftp_handle);

if (r == LIBSSH2_ERROR_EAGAIN || r == LIBSSH2_ERROR_TIMEOUT) {

@@ -XXX,XX +XXX,XX @@ static int64_t ssh_getlength(BlockDriverState *bs)

/* Note we cannot make a libssh2 call here. */

length = (int64_t) s->attrs.filesize;

- DPRINTF("length=%" PRIi64, length);

+ trace_ssh_getlength(length);

return length;

}

diff --git a/block/trace-events b/block/trace-events

index XXXXXXX..XXXXXXX 100644

--- a/block/trace-events

+++ b/block/trace-events

@@ -XXX,XX +XXX,XX @@ iscsi_xcopy(void *src_lun, uint64_t src_off, void *dst_lun, uint64_t dst_off, ui

# block/nbd-client.c

nbd_read_reply_entry_fail(int ret, const char *err) "ret = %d, err: %s"

nbd_co_request_fail(uint64_t from, uint32_t len, uint64_t handle, uint16_t flags, uint16_t type, const char *name, int ret, const char *err) "Request failed { .from = %" PRIu64", .len = %" PRIu32 ", .handle = %" PRIu64 ", .flags = 0x%" PRIx16 ", .type = %" PRIu16 " (%s) } ret = %d, err: %s"

+

+# block/ssh.c

+ssh_restart_coroutine(void *co) "co=%p"

+ssh_flush(void) "fsync"

+ssh_check_host_key_knownhosts(const char *key) "host key OK: %s"

+ssh_connect_to_ssh(char *path, int flags, int mode) "opening file %s flags=0x%x creat_mode=0%o"

+ssh_co_yield(int sock, void *rd_handler, void *wr_handler) "s->sock=%d rd_handler=%p wr_handler=%p"

+ssh_co_yield_back(int sock) "s->sock=%d - back"

+ssh_getlength(int64_t length) "length=%" PRIi64

+ssh_co_create_opts(uint64_t size) "total_size=%" PRIu64

+ssh_read(int64_t offset, size_t size) "offset=%" PRIi64 " size=%zu"

+ssh_read_buf(void *buf, size_t size) "sftp_read buf=%p size=%zu"

+ssh_read_return(ssize_t ret) "sftp_read returned %zd"

+ssh_write(int64_t offset, size_t size) "offset=%" PRIi64 " size=%zu"

+ssh_write_buf(void *buf, size_t size) "sftp_write buf=%p size=%zu"

+ssh_write_return(ssize_t ret) "sftp_write returned %zd"

+ssh_seek(int64_t offset) "seeking to offset=%" PRIi64

--

2.20.1

From: Laurent Vivier <lvivier@redhat.com>

Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: Richard W.M. Jones <rjones@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Message-id: 20181213162727.17438-3-lvivier@redhat.com

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

block/curl.c | 29 ++++++++---------------------

block/trace-events | 9 +++++++++

2 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/block/curl.c b/block/curl.c

index XXXXXXX..XXXXXXX 100644

--- a/block/curl.c

+++ b/block/curl.c

@@ -XXX,XX +XXX,XX @@

#include "crypto/secret.h"

#include <curl/curl.h>

#include "qemu/cutils.h"

+#include "trace.h"

-// #define DEBUG_CURL

// #define DEBUG_VERBOSE

-#ifdef DEBUG_CURL

-#define DEBUG_CURL_PRINT 1

-#else

-#define DEBUG_CURL_PRINT 0

-#endif

-#define DPRINTF(fmt, ...) \

- do { \

- if (DEBUG_CURL_PRINT) { \

- fprintf(stderr, fmt, ## __VA_ARGS__); \

- } \

- } while (0)

-

#if LIBCURL_VERSION_NUM >= 0x071000

/* The multi interface timer callback was introduced in 7.16.0 */

#define NEED_CURL_TIMER_CALLBACK

@@ -XXX,XX +XXX,XX @@ static int curl_timer_cb(CURLM *multi, long timeout_ms, void *opaque)

{

BDRVCURLState *s = opaque;

- DPRINTF("CURL: timer callback timeout_ms %ld\n", timeout_ms);

+ trace_curl_timer_cb(timeout_ms);

if (timeout_ms == -1) {

timer_del(&s->timer);

} else {

@@ -XXX,XX +XXX,XX @@ static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action,

}

socket = NULL;

- DPRINTF("CURL (AIO): Sock action %d on fd %d\n", action, (int)fd);

+ trace_curl_sock_cb(action, (int)fd);

switch (action) {

case CURL_POLL_IN:

aio_set_fd_handler(s->aio_context, fd, false,

@@ -XXX,XX +XXX,XX @@ static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void *opaque)

size_t realsize = size * nmemb;

int i;

- DPRINTF("CURL: Just reading %zd bytes\n", realsize);

+ trace_curl_read_cb(realsize);

if (!s || !s->orig_buf) {

goto read_end;

@@ -XXX,XX +XXX,XX @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,

}

}

- DPRINTF("CURL: Opening %s\n", file);

+ trace_curl_open(file);

qemu_co_queue_init(&s->free_state_waitq);

s->aio_context = bdrv_get_aio_context(bs);

s->url = g_strdup(file);

@@ -XXX,XX +XXX,XX @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,

"Server does not support 'range' (byte ranges).");

goto out;

}

- DPRINTF("CURL: Size = %" PRIu64 "\n", s->len);

+ trace_curl_open_size(s->len);

qemu_mutex_lock(&s->mutex);

curl_clean_state(state);

@@ -XXX,XX +XXX,XX @@ static void curl_setup_preadv(BlockDriverState *bs, CURLAIOCB *acb)

state->acb[0] = acb;

snprintf(state->range, 127, "%" PRIu64 "-%" PRIu64, start, end);

- DPRINTF("CURL (AIO): Reading %" PRIu64 " at %" PRIu64 " (%s)\n",

- acb->bytes, start, state->range);

+ trace_curl_setup_preadv(acb->bytes, start, state->range);

curl_easy_setopt(state->curl, CURLOPT_RANGE, state->range);

curl_multi_add_handle(s->multi, state->curl);

@@ -XXX,XX +XXX,XX @@ static void curl_close(BlockDriverState *bs)

{

BDRVCURLState *s = bs->opaque;

- DPRINTF("CURL: Close\n");

+ trace_curl_close();

curl_detach_aio_context(bs);

qemu_mutex_destroy(&s->mutex);

diff --git a/block/trace-events b/block/trace-events

index XXXXXXX..XXXXXXX 100644

--- a/block/trace-events

+++ b/block/trace-events

@@ -XXX,XX +XXX,XX @@ ssh_write(int64_t offset, size_t size) "offset=%" PRIi64 " size=%zu"

ssh_write_buf(void *buf, size_t size) "sftp_write buf=%p size=%zu"

ssh_write_return(ssize_t ret) "sftp_write returned %zd"

ssh_seek(int64_t offset) "seeking to offset=%" PRIi64

+

+# block/curl.c

+curl_timer_cb(long timeout_ms) "timer callback timeout_ms %ld"

+curl_sock_cb(int action, int fd) "sock action %d on fd %d"

+curl_read_cb(size_t realsize) "just reading %zu bytes"

+curl_open(const char *file) "opening %s"

+curl_open_size(uint64_t size) "size = %" PRIu64

+curl_setup_preadv(uint64_t bytes, uint64_t start, const char *range) "reading %" PRIu64 " at %" PRIu64 " (%s)"

+curl_close(void) "close"

--

2.20.1

From: Laurent Vivier <lvivier@redhat.com>

Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Message-id: 20181213162727.17438-4-lvivier@redhat.com

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

block/file-posix.c | 25 ++++++-------------------

block/trace-events | 7 +++++++

2 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c

index XXXXXXX..XXXXXXX 100644

--- a/block/file-posix.c

+++ b/block/file-posix.c

@@ -XXX,XX +XXX,XX @@

#include <xfs/xfs.h>

#endif

-//#define DEBUG_BLOCK

-

-#ifdef DEBUG_BLOCK

-# define DEBUG_BLOCK_PRINT 1

-#else

-# define DEBUG_BLOCK_PRINT 0

-#endif

-#define DPRINTF(fmt, ...) \

-do { \

- if (DEBUG_BLOCK_PRINT) { \

- printf(fmt, ## __VA_ARGS__); \

- } \

-} while (0)

+#include "trace.h"

/* OS X does not have O_DSYNC */

#ifndef O_DSYNC

@@ -XXX,XX +XXX,XX @@ static int xfs_write_zeroes(BDRVRawState *s, int64_t offset, uint64_t bytes)

if (xfsctl(NULL, s->fd, XFS_IOC_ZERO_RANGE, &fl) < 0) {

err = errno;

- DPRINTF("cannot write zero range (%s)\n", strerror(errno));

+ trace_file_xfs_write_zeroes(strerror(errno));

return -err;

}

@@ -XXX,XX +XXX,XX @@ static int xfs_discard(BDRVRawState *s, int64_t offset, uint64_t bytes)

if (xfsctl(NULL, s->fd, XFS_IOC_UNRESVSP64, &fl) < 0) {

err = errno;

- DPRINTF("cannot punch hole (%s)\n", strerror(errno));

+ trace_file_xfs_discard(strerror(errno));

return -err;

}

@@ -XXX,XX +XXX,XX @@ static char *FindEjectableOpticalMedia(io_iterator_t *mediaIterator)

/* If a match was found, leave the loop */

if (*mediaIterator != 0) {

- DPRINTF("Matching using %s\n", matching_array[index]);

+ trace_file_FindEjectableOpticalMedia(matching_array[index]);

mediaType = g_strdup(matching_array[index]);

break;

}

@@ -XXX,XX +XXX,XX @@ static bool setup_cdrom(char *bsd_path, Error **errp)

if (partition_found == false) {

error_setg(errp, "Failed to find a working partition on disc");

} else {

- DPRINTF("Using %s as optical disc\n", test_partition);

+ trace_file_setup_cdrom(test_partition);

pstrcpy(bsd_path, MAXPATHLEN, test_partition);

}

return partition_found;

@@ -XXX,XX +XXX,XX @@ static bool hdev_is_sg(BlockDriverState *bs)

ret = ioctl(s->fd, SG_GET_SCSI_ID, &scsiid);

if (ret >= 0) {

- DPRINTF("SG device found: type=%d, version=%d\n",

- scsiid.scsi_type, sg_version);

+ trace_file_hdev_is_sg(scsiid.scsi_type, sg_version);

return true;

}

diff --git a/block/trace-events b/block/trace-events

index XXXXXXX..XXXXXXX 100644

--- a/block/trace-events

+++ b/block/trace-events

@@ -XXX,XX +XXX,XX @@ curl_open(const char *file) "opening %s"

curl_open_size(uint64_t size) "size = %" PRIu64

curl_setup_preadv(uint64_t bytes, uint64_t start, const char *range) "reading %" PRIu64 " at %" PRIu64 " (%s)"

curl_close(void) "close"

+

+# block/file-posix.c

+file_xfs_write_zeroes(const char *error) "cannot write zero range (%s)"

+file_xfs_discard(const char *error) "cannot punch hole (%s)"

+file_FindEjectableOpticalMedia(const char *media) "Matching using %s"

+file_setup_cdrom(const char *partition) "Using %s as optical disc"

+file_hdev_is_sg(int type, int version) "SG device found: type=%d, version=%d"

--

2.20.1

From: Laurent Vivier <lvivier@redhat.com>

Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Message-id: 20181213162727.17438-5-lvivier@redhat.com

[mreitz: Fixed sheepdog_snapshot_create_inode's format string to use

PRIx32 for uint32_ts]

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

block/sheepdog.c | 47 +++++++++++++++++-----------------------------

block/trace-events | 14 ++++++++++++++

2 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c

index XXXXXXX..XXXXXXX 100644

--- a/block/sheepdog.c

+++ b/block/sheepdog.c

@@ -XXX,XX +XXX,XX @@

#include "sysemu/block-backend.h"

#include "qemu/bitops.h"

#include "qemu/cutils.h"

+#include "trace.h"

#define SD_PROTO_VER 0x01

@@ -XXX,XX +XXX,XX @@ static inline size_t count_data_objs(const struct SheepdogInode *inode)

(1UL << inode->block_size_shift));

}

-#undef DPRINTF

-#ifdef DEBUG_SDOG

-#define DEBUG_SDOG_PRINT 1

-#else

-#define DEBUG_SDOG_PRINT 0

-#endif

-#define DPRINTF(fmt, args...) \

- do { \

- if (DEBUG_SDOG_PRINT) { \

- fprintf(stderr, "%s %d: " fmt, __func__, __LINE__, ##args); \

- } \

- } while (0)

-

typedef struct SheepdogAIOCB SheepdogAIOCB;

typedef struct BDRVSheepdogState BDRVSheepdogState;

@@ -XXX,XX +XXX,XX @@ static coroutine_fn void reconnect_to_sdog(void *opaque)

Error *local_err = NULL;

s->fd = get_sheep_fd(s, &local_err);

if (s->fd < 0) {

- DPRINTF("Wait for connection to be established\n");

+ trace_sheepdog_reconnect_to_sdog();

error_report_err(local_err);

qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, 1000000000ULL);

}

@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)

break;

case AIOCB_FLUSH_CACHE:

if (rsp.result == SD_RES_INVALID_PARMS) {

- DPRINTF("disable cache since the server doesn't support it\n");

+ trace_sheepdog_aio_read_response();

s->cache_flags = SD_FLAG_CMD_DIRECT;

rsp.result = SD_RES_SUCCESS;

}

@@ -XXX,XX +XXX,XX @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags,

s->discard_supported = true;

if (snap_id || tag[0]) {

- DPRINTF("%" PRIx32 " snapshot inode was open.\n", vid);

+ trace_sheepdog_open(vid);

s->is_snapshot = true;

}

@@ -XXX,XX +XXX,XX @@ static void sd_close(BlockDriverState *bs)

unsigned int wlen, rlen = 0;

int fd, ret;

- DPRINTF("%s\n", s->name);

+ trace_sheepdog_close(s->name);

fd = connect_to_sdog(s, &local_err);

if (fd < 0) {

@@ -XXX,XX +XXX,XX @@ static int sd_create_branch(BDRVSheepdogState *s)

char *buf;

bool deleted;

- DPRINTF("%" PRIx32 " is snapshot.\n", s->inode.vdi_id);

+ trace_sheepdog_create_branch_snapshot(s->inode.vdi_id);

buf = g_malloc(SD_INODE_SIZE);

@@ -XXX,XX +XXX,XX @@ static int sd_create_branch(BDRVSheepdogState *s)

goto out;

}

- DPRINTF("%" PRIx32 " is created.\n", vid);

+ trace_sheepdog_create_branch_created(vid);

fd = connect_to_sdog(s, &local_err);

if (fd < 0) {

@@ -XXX,XX +XXX,XX @@ static int sd_create_branch(BDRVSheepdogState *s)

s->is_snapshot = false;

ret = 0;

- DPRINTF("%" PRIx32 " was newly created.\n", s->inode.vdi_id);

+ trace_sheepdog_create_branch_new(s->inode.vdi_id);

out:

g_free(buf);

@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_co_rw_vector(SheepdogAIOCB *acb)

}

if (create) {

- DPRINTF("update ino (%" PRIu32 ") %" PRIu64 " %" PRIu64 " %ld\n",

- inode->vdi_id, oid,

- vid_to_data_oid(inode->data_vdi_id[idx], idx), idx);

+ trace_sheepdog_co_rw_vector_update(inode->vdi_id, oid,

+ vid_to_data_oid(inode->data_vdi_id[idx], idx),

+ idx);

oid = vid_to_data_oid(inode->vdi_id, idx);

- DPRINTF("new oid %" PRIx64 "\n", oid);

+ trace_sheepdog_co_rw_vector_new(oid);

}

aio_req = alloc_aio_req(s, acb, oid, len, offset, flags, create,

@@ -XXX,XX +XXX,XX @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)

SheepdogInode *inode;

unsigned int datalen;

- DPRINTF("sn_info: name %s id_str %s s: name %s vm_state_size %" PRId64 " "

- "is_snapshot %d\n", sn_info->name, sn_info->id_str,

- s->name, sn_info->vm_state_size, s->is_snapshot);

+ trace_sheepdog_snapshot_create_info(sn_info->name, sn_info->id_str, s->name,

+ sn_info->vm_state_size, s->is_snapshot);

if (s->is_snapshot) {

error_report("You can't create a snapshot of a snapshot VDI, "

@@ -XXX,XX +XXX,XX @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)

return -EINVAL;

}

- DPRINTF("%s %s\n", sn_info->name, sn_info->id_str);

+ trace_sheepdog_snapshot_create(sn_info->name, sn_info->id_str);

s->inode.vm_state_size = sn_info->vm_state_size;

s->inode.vm_clock_nsec = sn_info->vm_clock_nsec;

@@ -XXX,XX +XXX,XX @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)

}

memcpy(&s->inode, inode, datalen);

- DPRINTF("s->inode: name %s snap_id %x oid %x\n",

- s->inode.name, s->inode.snap_id, s->inode.vdi_id);

+ trace_sheepdog_snapshot_create_inode(s->inode.name, s->inode.snap_id,

+ s->inode.vdi_id);

cleanup:

g_free(inode);

diff --git a/block/trace-events b/block/trace-events

index XXXXXXX..XXXXXXX 100644

--- a/block/trace-events

+++ b/block/trace-events

@@ -XXX,XX +XXX,XX @@ file_xfs_discard(const char *error) "cannot punch hole (%s)"

file_FindEjectableOpticalMedia(const char *media) "Matching using %s"

file_setup_cdrom(const char *partition) "Using %s as optical disc"

file_hdev_is_sg(int type, int version) "SG device found: type=%d, version=%d"

+

+# block/sheepdog.c

+sheepdog_reconnect_to_sdog(void) "Wait for connection to be established"

+sheepdog_aio_read_response(void) "disable cache since the server doesn't support it"

+sheepdog_open(uint32_t vid) "0x%" PRIx32 " snapshot inode was open"

+sheepdog_close(const char *name) "%s"

+sheepdog_create_branch_snapshot(uint32_t vdi) "0x%" PRIx32 " is snapshot"

+sheepdog_create_branch_created(uint32_t vdi) "0x%" PRIx32 " is created"

+sheepdog_create_branch_new(uint32_t vdi) "0x%" PRIx32 " was newly created"

+sheepdog_co_rw_vector_update(uint32_t vdi, uint64_t oid, uint64_t data, long idx) "update ino (%" PRIu32 ") %" PRIu64 " %" PRIu64 " %ld"

+sheepdog_co_rw_vector_new(uint64_t oid) "new oid 0x%" PRIx64

+sheepdog_snapshot_create_info(const char *sn_name, const char *id, const char *name, int64_t size, int is_snapshot) "sn_info: name %s id_str %s s: name %s vm_state_size %" PRId64 " " "is_snapshot %d"

+sheepdog_snapshot_create(const char *sn_name, const char *id) "%s %s"

+sheepdog_snapshot_create_inode(const char *name, uint32_t snap, uint32_t vdi) "s->inode: name %s snap_id 0x%" PRIx32 " vdi 0x%" PRIx32

--

2.20.1

From: "Richard W.M. Jones" <rjones@redhat.com>

On FreeBSD 11.2:

$ nbdkit memory size=1M --run './qemu-io -f raw -c "aio_write 0 512" $nbd'

Parsing error: non-numeric argument, or extraneous/unrecognized suffix -- aio_write

After main option parsing, we reinitialize optind so we can parse each

command. However reinitializing optind to 0 does not work on FreeBSD.

What happens when you do this is optind remains 0 after the option

parsing loop, and the result is we try to parse argv[optind] ==

argv[0] == "aio_write" as if it was the first parameter.

The FreeBSD manual page says:

In order to use getopt() to evaluate multiple sets of arguments, or to

evaluate a single set of arguments multiple times, the variable optreset

must be set to 1 before the second and each additional set of calls to

getopt(), and the variable optind must be reinitialized.

(From the rest of the man page it is clear that optind must be

reinitialized to 1).

The glibc man page says:

A program that scans multiple argument vectors, or rescans the same

vector more than once, and wants to make use of GNU extensions such as

'+' and '-' at the start of optstring, or changes the value of

POSIXLY_CORRECT between scans, must reinitialize getopt() by resetting

optind to 0, rather than the traditional value of 1. (Resetting to 0

forces the invocation of an internal initialization routine that

rechecks POSIXLY_CORRECT and checks for GNU extensions in optstring.)

This commit introduces an OS-portability function called

qemu_reset_optind which provides a way of resetting optind that works

on FreeBSD and platforms that use optreset, while keeping it the same

as now on other platforms.

Note that the qemu codebase sets optind in many other places, but in

those other places it's setting a local variable and not using getopt.

This change is only needed in places where we are using getopt and the

associated global variable optind.

Signed-off-by: Richard W.M. Jones <rjones@redhat.com>

Message-id: 20190118101114.11759-2-rjones@redhat.com

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Reviewed-by: Eric Blake <eblake@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

configure | 14 ++++++++++++++

include/qemu/osdep.h | 16 ++++++++++++++++

qemu-img.c | 2 +-

qemu-io-cmds.c | 2 +-

4 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/configure b/configure

index XXXXXXX..XXXXXXX 100755

--- a/configure

+++ b/configure

@@ -XXX,XX +XXX,XX @@ if compile_prog "" "" ; then

signalfd=yes

fi

+# check if optreset global is declared by <getopt.h>

+optreset="no"

+cat > $TMPC << EOF

+#include <getopt.h>

+int main(void) { return optreset; }

+EOF

+

+if compile_prog "" "" ; then

+ optreset=yes

+fi

+

# check if eventfd is supported

eventfd=no

cat > $TMPC << EOF

@@ -XXX,XX +XXX,XX @@ fi

if test "$signalfd" = "yes" ; then

echo "CONFIG_SIGNALFD=y" >> $config_host_mak

fi

+if test "$optreset" = "yes" ; then

+ echo "HAVE_OPTRESET=y" >> $config_host_mak

+fi

if test "$tcg" = "yes"; then

echo "CONFIG_TCG=y" >> $config_host_mak

if test "$tcg_interpreter" = "yes" ; then

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h

index XXXXXXX..XXXXXXX 100644

--- a/include/qemu/osdep.h

+++ b/include/qemu/osdep.h

@@ -XXX,XX +XXX,XX @@ extern int daemon(int, int);

#include <ctype.h>

#include <errno.h>

#include <fcntl.h>

+#include <getopt.h>

#include <sys/stat.h>

#include <sys/time.h>

#include <assert.h>

@@ -XXX,XX +XXX,XX @@ extern int qemu_icache_linesize_log;

extern int qemu_dcache_linesize;

extern int qemu_dcache_linesize_log;

+/*

+ * After using getopt or getopt_long, if you need to parse another set

+ * of options, then you must reset optind. Unfortunately the way to

+ * do this varies between implementations of getopt.

+ */

+static inline void qemu_reset_optind(void)

+{

+#ifdef HAVE_OPTRESET

+ optind = 1;

+ optreset = 1;

+#else

+ optind = 0;

+#endif

+}

+

#endif

diff --git a/qemu-img.c b/qemu-img.c

index XXXXXXX..XXXXXXX 100644

--- a/qemu-img.c

+++ b/qemu-img.c

@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv)

return 0;

}

argv += optind;

- optind = 0;

+ qemu_reset_optind();

if (!trace_init_backends()) {

exit(1);

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c

index XXXXXXX..XXXXXXX 100644