* [Qemu-devel] [PATCH 1/3] blockdev: Rename drive_init(), drive_uninit() to drive_new(), drive_del()
2014-06-06 12:50 [Qemu-devel] [PATCH 0/3] block: Minor cleanups Markus Armbruster
@ 2014-06-06 12:50 ` Markus Armbruster
2014-06-06 14:05 ` Benoît Canet
2014-06-06 12:50 ` [Qemu-devel] [PATCH 2/3] blockdev: Remove unused DriveInfo reference count Markus Armbruster
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Markus Armbruster @ 2014-06-06 12:50 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, famz, stefanha
"Init" and "uninit" suggest the functions don't allocate / free
storage. But they do.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
blockdev.c | 14 +++++++-------
device-hotplug.c | 2 +-
hw/usb/dev-storage.c | 2 +-
include/sysemu/blockdev.h | 4 ++--
vl.c | 4 ++--
5 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 9b5261b..1256013 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -213,7 +213,7 @@ static void bdrv_format_print(void *opaque, const char *name)
error_printf(" %s", name);
}
-static void drive_uninit(DriveInfo *dinfo)
+static void drive_del(DriveInfo *dinfo)
{
if (dinfo->opts) {
qemu_opts_del(dinfo->opts);
@@ -230,7 +230,7 @@ void drive_put_ref(DriveInfo *dinfo)
{
assert(dinfo->refcount);
if (--dinfo->refcount == 0) {
- drive_uninit(dinfo);
+ drive_del(dinfo);
}
}
@@ -658,7 +658,7 @@ QemuOptsList qemu_legacy_drive_opts = {
},
};
-DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
+DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
{
const char *value;
DriveInfo *dinfo = NULL;
@@ -1791,7 +1791,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
bdrv_set_on_error(bs, BLOCKDEV_ON_ERROR_REPORT,
BLOCKDEV_ON_ERROR_REPORT);
} else {
- drive_uninit(drive_get_by_blockdev(bs));
+ drive_del(drive_get_by_blockdev(bs));
}
return 0;
@@ -2334,9 +2334,9 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
goto fail;
}
- /* TODO Sort it out in raw-posix and drive_init: Reject aio=native with
+ /* TODO Sort it out in raw-posix and drive_new(): Reject aio=native with
* cache.direct=false instead of silently switching to aio=threads, except
- * if called from drive_init.
+ * when called from drive_new().
*
* For now, simply forbidding the combination for all drivers will do. */
if (options->has_aio && options->aio == BLOCKDEV_AIO_OPTIONS_NATIVE) {
@@ -2368,7 +2368,7 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
}
if (bdrv_key_required(dinfo->bdrv)) {
- drive_uninit(dinfo);
+ drive_del(dinfo);
error_setg(errp, "blockdev-add doesn't support encrypted devices");
goto fail;
}
diff --git a/device-hotplug.c b/device-hotplug.c
index eecb08e..fc09d10 100644
--- a/device-hotplug.c
+++ b/device-hotplug.c
@@ -40,7 +40,7 @@ DriveInfo *add_init_drive(const char *optstr)
return NULL;
mc = MACHINE_GET_CLASS(current_machine);
- dinfo = drive_init(opts, mc->block_default_type);
+ dinfo = drive_new(opts, mc->block_default_type);
if (!dinfo) {
qemu_opts_del(opts);
return NULL;
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index e919100..ae4efcb 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -691,7 +691,7 @@ static USBDevice *usb_msd_init(USBBus *bus, const char *filename)
qemu_opt_set(opts, "if", "none");
/* create host drive */
- dinfo = drive_init(opts, 0);
+ dinfo = drive_new(opts, 0);
if (!dinfo) {
qemu_opts_del(opts);
return NULL;
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 134712b..0fdbd68 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -37,7 +37,7 @@ struct DriveInfo {
int bus;
int unit;
int auto_del; /* see blockdev_mark_auto_del() */
- bool enable_auto_del; /* Only for legacy drive_init() */
+ bool enable_auto_del; /* Only for legacy drive_new() */
int media_cd;
int cyls, heads, secs, trans;
QemuOpts *opts;
@@ -57,7 +57,7 @@ DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
QemuOpts *drive_def(const char *optstr);
QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
const char *optstr);
-DriveInfo *drive_init(QemuOpts *arg, BlockInterfaceType block_default_type);
+DriveInfo *drive_new(QemuOpts *arg, BlockInterfaceType block_default_type);
/* device-hotplug */
diff --git a/vl.c b/vl.c
index a3def97..f4b4841 100644
--- a/vl.c
+++ b/vl.c
@@ -1074,7 +1074,7 @@ static int drive_init_func(QemuOpts *opts, void *opaque)
{
BlockInterfaceType *block_default_type = opaque;
- return drive_init(opts, *block_default_type) == NULL;
+ return drive_new(opts, *block_default_type) == NULL;
}
static int drive_enable_snapshot(QemuOpts *opts, void *opaque)
@@ -1098,7 +1098,7 @@ static void default_drive(int enable, int snapshot, BlockInterfaceType type,
if (snapshot) {
drive_enable_snapshot(opts, NULL);
}
- if (!drive_init(opts, type)) {
+ if (!drive_new(opts, type)) {
exit(1);
}
}
--
1.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH 1/3] blockdev: Rename drive_init(), drive_uninit() to drive_new(), drive_del()
2014-06-06 12:50 ` [Qemu-devel] [PATCH 1/3] blockdev: Rename drive_init(), drive_uninit() to drive_new(), drive_del() Markus Armbruster
@ 2014-06-06 14:05 ` Benoît Canet
2014-06-06 16:13 ` Markus Armbruster
0 siblings, 1 reply; 10+ messages in thread
From: Benoît Canet @ 2014-06-06 14:05 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kwolf, famz, qemu-devel, stefanha
The Friday 06 Jun 2014 à 14:50:58 (+0200), Markus Armbruster wrote :
> "Init" and "uninit" suggest the functions don't allocate / free
> storage. But they do.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> blockdev.c | 14 +++++++-------
> device-hotplug.c | 2 +-
> hw/usb/dev-storage.c | 2 +-
> include/sysemu/blockdev.h | 4 ++--
> vl.c | 4 ++--
> 5 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 9b5261b..1256013 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -213,7 +213,7 @@ static void bdrv_format_print(void *opaque, const char *name)
> error_printf(" %s", name);
> }
>
> -static void drive_uninit(DriveInfo *dinfo)
> +static void drive_del(DriveInfo *dinfo)
> {
> if (dinfo->opts) {
> qemu_opts_del(dinfo->opts);
> @@ -230,7 +230,7 @@ void drive_put_ref(DriveInfo *dinfo)
> {
> assert(dinfo->refcount);
> if (--dinfo->refcount == 0) {
> - drive_uninit(dinfo);
> + drive_del(dinfo);
> }
> }
>
> @@ -658,7 +658,7 @@ QemuOptsList qemu_legacy_drive_opts = {
> },
> };
>
> -DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
> +DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
> {
> const char *value;
> DriveInfo *dinfo = NULL;
> @@ -1791,7 +1791,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
> bdrv_set_on_error(bs, BLOCKDEV_ON_ERROR_REPORT,
> BLOCKDEV_ON_ERROR_REPORT);
> } else {
> - drive_uninit(drive_get_by_blockdev(bs));
> + drive_del(drive_get_by_blockdev(bs));
> }
>
> return 0;
> @@ -2334,9 +2334,9 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
> goto fail;
> }
>
> - /* TODO Sort it out in raw-posix and drive_init: Reject aio=native with
> + /* TODO Sort it out in raw-posix and drive_new(): Reject aio=native with
> * cache.direct=false instead of silently switching to aio=threads, except
> - * if called from drive_init.
> + * when called from drive_new().
> *
> * For now, simply forbidding the combination for all drivers will do. */
> if (options->has_aio && options->aio == BLOCKDEV_AIO_OPTIONS_NATIVE) {
> @@ -2368,7 +2368,7 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
> }
>
> if (bdrv_key_required(dinfo->bdrv)) {
> - drive_uninit(dinfo);
> + drive_del(dinfo);
> error_setg(errp, "blockdev-add doesn't support encrypted devices");
> goto fail;
> }
> diff --git a/device-hotplug.c b/device-hotplug.c
> index eecb08e..fc09d10 100644
> --- a/device-hotplug.c
> +++ b/device-hotplug.c
> @@ -40,7 +40,7 @@ DriveInfo *add_init_drive(const char *optstr)
> return NULL;
>
> mc = MACHINE_GET_CLASS(current_machine);
> - dinfo = drive_init(opts, mc->block_default_type);
> + dinfo = drive_new(opts, mc->block_default_type);
> if (!dinfo) {
> qemu_opts_del(opts);
> return NULL;
> diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
> index e919100..ae4efcb 100644
> --- a/hw/usb/dev-storage.c
> +++ b/hw/usb/dev-storage.c
> @@ -691,7 +691,7 @@ static USBDevice *usb_msd_init(USBBus *bus, const char *filename)
> qemu_opt_set(opts, "if", "none");
>
> /* create host drive */
> - dinfo = drive_init(opts, 0);
> + dinfo = drive_new(opts, 0);
> if (!dinfo) {
> qemu_opts_del(opts);
> return NULL;
> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> index 134712b..0fdbd68 100644
> --- a/include/sysemu/blockdev.h
> +++ b/include/sysemu/blockdev.h
> @@ -37,7 +37,7 @@ struct DriveInfo {
> int bus;
> int unit;
> int auto_del; /* see blockdev_mark_auto_del() */
> - bool enable_auto_del; /* Only for legacy drive_init() */
> + bool enable_auto_del; /* Only for legacy drive_new() */
> int media_cd;
> int cyls, heads, secs, trans;
> QemuOpts *opts;
> @@ -57,7 +57,7 @@ DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
> QemuOpts *drive_def(const char *optstr);
> QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
> const char *optstr);
> -DriveInfo *drive_init(QemuOpts *arg, BlockInterfaceType block_default_type);
> +DriveInfo *drive_new(QemuOpts *arg, BlockInterfaceType block_default_type);
>
> /* device-hotplug */
>
> diff --git a/vl.c b/vl.c
gG> index a3def97..f4b4841 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1074,7 +1074,7 @@ static int drive_init_func(QemuOpts *opts, void *opaque)
> {
> BlockInterfaceType *block_default_type = opaque;
>
> - return drive_init(opts, *block_default_type) == NULL;
> + return drive_new(opts, *block_default_type) == NULL;
> }
>
> static int drive_enable_snapshot(QemuOpts *opts, void *opaque)
> @@ -1098,7 +1098,7 @@ static void default_drive(int enable, int snapshot, BlockInterfaceType type,
> if (snapshot) {
> drive_enable_snapshot(opts, NULL);
> }
> - if (!drive_init(opts, type)) {
> + if (!drive_new(opts, type)) {
Unrelated but it seems QEMU is leaking this one on exit.
> exit(1);
> }
> }
> --
> 1.9.3
>
>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH 1/3] blockdev: Rename drive_init(), drive_uninit() to drive_new(), drive_del()
2014-06-06 14:05 ` Benoît Canet
@ 2014-06-06 16:13 ` Markus Armbruster
0 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2014-06-06 16:13 UTC (permalink / raw)
To: Benoît Canet; +Cc: kwolf, famz, qemu-devel, stefanha
Benoît Canet <benoit.canet@irqsave.net> writes:
> The Friday 06 Jun 2014 à 14:50:58 (+0200), Markus Armbruster wrote :
>> "Init" and "uninit" suggest the functions don't allocate / free
>> storage. But they do.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> blockdev.c | 14 +++++++-------
>> device-hotplug.c | 2 +-
>> hw/usb/dev-storage.c | 2 +-
>> include/sysemu/blockdev.h | 4 ++--
>> vl.c | 4 ++--
>> 5 files changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/blockdev.c b/blockdev.c
>> index 9b5261b..1256013 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -213,7 +213,7 @@ static void bdrv_format_print(void *opaque, const char *name)
>> error_printf(" %s", name);
>> }
>>
>> -static void drive_uninit(DriveInfo *dinfo)
>> +static void drive_del(DriveInfo *dinfo)
>> {
>> if (dinfo->opts) {
>> qemu_opts_del(dinfo->opts);
>> @@ -230,7 +230,7 @@ void drive_put_ref(DriveInfo *dinfo)
>> {
>> assert(dinfo->refcount);
>> if (--dinfo->refcount == 0) {
>> - drive_uninit(dinfo);
>> + drive_del(dinfo);
>> }
>> }
>>
>> @@ -658,7 +658,7 @@ QemuOptsList qemu_legacy_drive_opts = {
>> },
>> };
>>
>> -DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
>> +DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
>> {
>> const char *value;
>> DriveInfo *dinfo = NULL;
>> @@ -1791,7 +1791,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
>> bdrv_set_on_error(bs, BLOCKDEV_ON_ERROR_REPORT,
>> BLOCKDEV_ON_ERROR_REPORT);
>> } else {
>> - drive_uninit(drive_get_by_blockdev(bs));
>> + drive_del(drive_get_by_blockdev(bs));
>> }
>>
>> return 0;
>> @@ -2334,9 +2334,9 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
>> goto fail;
>> }
>>
>> - /* TODO Sort it out in raw-posix and drive_init: Reject aio=native with
>> + /* TODO Sort it out in raw-posix and drive_new(): Reject aio=native with
>> * cache.direct=false instead of silently switching to aio=threads, except
>> - * if called from drive_init.
>> + * when called from drive_new().
>> *
>> * For now, simply forbidding the combination for all drivers will do. */
>> if (options->has_aio && options->aio == BLOCKDEV_AIO_OPTIONS_NATIVE) {
>> @@ -2368,7 +2368,7 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
>> }
>>
>> if (bdrv_key_required(dinfo->bdrv)) {
>> - drive_uninit(dinfo);
>> + drive_del(dinfo);
>> error_setg(errp, "blockdev-add doesn't support encrypted devices");
>> goto fail;
>> }
>> diff --git a/device-hotplug.c b/device-hotplug.c
>> index eecb08e..fc09d10 100644
>> --- a/device-hotplug.c
>> +++ b/device-hotplug.c
>> @@ -40,7 +40,7 @@ DriveInfo *add_init_drive(const char *optstr)
>> return NULL;
>>
>> mc = MACHINE_GET_CLASS(current_machine);
>> - dinfo = drive_init(opts, mc->block_default_type);
>> + dinfo = drive_new(opts, mc->block_default_type);
>> if (!dinfo) {
>> qemu_opts_del(opts);
>> return NULL;
>> diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
>> index e919100..ae4efcb 100644
>> --- a/hw/usb/dev-storage.c
>> +++ b/hw/usb/dev-storage.c
>> @@ -691,7 +691,7 @@ static USBDevice *usb_msd_init(USBBus *bus, const char *filename)
>> qemu_opt_set(opts, "if", "none");
>>
>> /* create host drive */
>> - dinfo = drive_init(opts, 0);
>> + dinfo = drive_new(opts, 0);
>> if (!dinfo) {
>> qemu_opts_del(opts);
>> return NULL;
>> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
>> index 134712b..0fdbd68 100644
>> --- a/include/sysemu/blockdev.h
>> +++ b/include/sysemu/blockdev.h
>> @@ -37,7 +37,7 @@ struct DriveInfo {
>> int bus;
>> int unit;
>> int auto_del; /* see blockdev_mark_auto_del() */
>> - bool enable_auto_del; /* Only for legacy drive_init() */
>> + bool enable_auto_del; /* Only for legacy drive_new() */
>> int media_cd;
>> int cyls, heads, secs, trans;
>> QemuOpts *opts;
>> @@ -57,7 +57,7 @@ DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
>> QemuOpts *drive_def(const char *optstr);
>> QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
>> const char *optstr);
>> -DriveInfo *drive_init(QemuOpts *arg, BlockInterfaceType block_default_type);
>> +DriveInfo *drive_new(QemuOpts *arg, BlockInterfaceType block_default_type);
>>
>> /* device-hotplug */
>>
>> diff --git a/vl.c b/vl.c
> gG> index a3def97..f4b4841 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1074,7 +1074,7 @@ static int drive_init_func(QemuOpts *opts, void *opaque)
>> {
>> BlockInterfaceType *block_default_type = opaque;
>>
>> - return drive_init(opts, *block_default_type) == NULL;
>> + return drive_new(opts, *block_default_type) == NULL;
>> }
>>
>> static int drive_enable_snapshot(QemuOpts *opts, void *opaque)
>> @@ -1098,7 +1098,7 @@ static void default_drive(int enable, int snapshot, BlockInterfaceType type,
>> if (snapshot) {
>> drive_enable_snapshot(opts, NULL);
>> }
>> - if (!drive_init(opts, type)) {
>> + if (!drive_new(opts, type)) {
>
> Unrelated but it seems QEMU is leaking this one on exit.
The kernel cleans it up for free, along with zillions of other unfreed
pieces of memory, file descriptors, and more.
>
>> exit(1);
>> }
>> }
>> --
>> 1.9.3
>>
>>
>
> Reviewed-by: Benoit Canet <benoit@irqsave.net>
Thanks!
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 2/3] blockdev: Remove unused DriveInfo reference count
2014-06-06 12:50 [Qemu-devel] [PATCH 0/3] block: Minor cleanups Markus Armbruster
2014-06-06 12:50 ` [Qemu-devel] [PATCH 1/3] blockdev: Rename drive_init(), drive_uninit() to drive_new(), drive_del() Markus Armbruster
@ 2014-06-06 12:50 ` Markus Armbruster
2014-06-06 14:09 ` Benoît Canet
2014-06-06 12:51 ` [Qemu-devel] [PATCH 3/3] block/qapi: Give some functions internal linkage Markus Armbruster
2014-06-11 10:36 ` [Qemu-devel] [PATCH 0/3] block: Minor cleanups Stefan Hajnoczi
3 siblings, 1 reply; 10+ messages in thread
From: Markus Armbruster @ 2014-06-06 12:50 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, famz, stefanha
It's always one since commit fa510eb dropped the last drive_get_ref().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
blockdev.c | 18 ++----------------
device-hotplug.c | 2 +-
hw/ide/piix.c | 2 +-
include/sysemu/blockdev.h | 4 +---
4 files changed, 5 insertions(+), 21 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 1256013..2e855d2 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -106,7 +106,7 @@ void blockdev_auto_del(BlockDriverState *bs)
DriveInfo *dinfo = drive_get_by_blockdev(bs);
if (dinfo && dinfo->auto_del) {
- drive_put_ref(dinfo);
+ drive_del(dinfo);
}
}
@@ -213,7 +213,7 @@ static void bdrv_format_print(void *opaque, const char *name)
error_printf(" %s", name);
}
-static void drive_del(DriveInfo *dinfo)
+void drive_del(DriveInfo *dinfo)
{
if (dinfo->opts) {
qemu_opts_del(dinfo->opts);
@@ -226,19 +226,6 @@ static void drive_del(DriveInfo *dinfo)
g_free(dinfo);
}
-void drive_put_ref(DriveInfo *dinfo)
-{
- assert(dinfo->refcount);
- if (--dinfo->refcount == 0) {
- drive_del(dinfo);
- }
-}
-
-void drive_get_ref(DriveInfo *dinfo)
-{
- dinfo->refcount++;
-}
-
typedef struct {
QEMUBH *bh;
BlockDriverState *bs;
@@ -500,7 +487,6 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
dinfo->bdrv->open_flags = snapshot ? BDRV_O_SNAPSHOT : 0;
dinfo->bdrv->read_only = ro;
dinfo->bdrv->detect_zeroes = detect_zeroes;
- dinfo->refcount = 1;
if (serial != NULL) {
dinfo->serial = g_strdup(serial);
}
diff --git a/device-hotplug.c b/device-hotplug.c
index fc09d10..e6a1ffb 100644
--- a/device-hotplug.c
+++ b/device-hotplug.c
@@ -76,6 +76,6 @@ void drive_hot_add(Monitor *mon, const QDict *qdict)
err:
if (dinfo) {
- drive_put_ref(dinfo);
+ drive_del(dinfo);
}
}
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 40757eb..8651726 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -184,7 +184,7 @@ int pci_piix3_xen_ide_unplug(DeviceState *dev)
}
bdrv_close(di->bdrv);
pci_ide->bus[di->bus].ifs[di->unit].bs = NULL;
- drive_put_ref(di);
+ drive_del(di);
}
}
qdev_reset_all(DEVICE(dev));
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 0fdbd68..23a5d10 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -43,21 +43,19 @@ struct DriveInfo {
QemuOpts *opts;
char *serial;
QTAILQ_ENTRY(DriveInfo) next;
- int refcount;
};
DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
DriveInfo *drive_get_by_index(BlockInterfaceType type, int index);
int drive_get_max_bus(BlockInterfaceType type);
DriveInfo *drive_get_next(BlockInterfaceType type);
-void drive_get_ref(DriveInfo *dinfo);
-void drive_put_ref(DriveInfo *dinfo);
DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
QemuOpts *drive_def(const char *optstr);
QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
const char *optstr);
DriveInfo *drive_new(QemuOpts *arg, BlockInterfaceType block_default_type);
+void drive_del(DriveInfo *dinfo);
/* device-hotplug */
--
1.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH 2/3] blockdev: Remove unused DriveInfo reference count
2014-06-06 12:50 ` [Qemu-devel] [PATCH 2/3] blockdev: Remove unused DriveInfo reference count Markus Armbruster
@ 2014-06-06 14:09 ` Benoît Canet
0 siblings, 0 replies; 10+ messages in thread
From: Benoît Canet @ 2014-06-06 14:09 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kwolf, famz, qemu-devel, stefanha
The Friday 06 Jun 2014 à 14:50:59 (+0200), Markus Armbruster wrote :
> It's always one since commit fa510eb dropped the last drive_get_ref().
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> blockdev.c | 18 ++----------------
> device-hotplug.c | 2 +-
> hw/ide/piix.c | 2 +-
> include/sysemu/blockdev.h | 4 +---
> 4 files changed, 5 insertions(+), 21 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 1256013..2e855d2 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -106,7 +106,7 @@ void blockdev_auto_del(BlockDriverState *bs)
> DriveInfo *dinfo = drive_get_by_blockdev(bs);
>
> if (dinfo && dinfo->auto_del) {
> - drive_put_ref(dinfo);
> + drive_del(dinfo);
> }
> }
>
> @@ -213,7 +213,7 @@ static void bdrv_format_print(void *opaque, const char *name)
> error_printf(" %s", name);
> }
>
> -static void drive_del(DriveInfo *dinfo)
> +void drive_del(DriveInfo *dinfo)
> {
> if (dinfo->opts) {
> qemu_opts_del(dinfo->opts);
> @@ -226,19 +226,6 @@ static void drive_del(DriveInfo *dinfo)
> g_free(dinfo);
> }
>
> -void drive_put_ref(DriveInfo *dinfo)
> -{
> - assert(dinfo->refcount);
> - if (--dinfo->refcount == 0) {
> - drive_del(dinfo);
> - }
> -}
> -
> -void drive_get_ref(DriveInfo *dinfo)
> -{
> - dinfo->refcount++;
> -}
> -
> typedef struct {
> QEMUBH *bh;
> BlockDriverState *bs;
> @@ -500,7 +487,6 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
> dinfo->bdrv->open_flags = snapshot ? BDRV_O_SNAPSHOT : 0;
> dinfo->bdrv->read_only = ro;
> dinfo->bdrv->detect_zeroes = detect_zeroes;
> - dinfo->refcount = 1;
> if (serial != NULL) {
> dinfo->serial = g_strdup(serial);
> }
> diff --git a/device-hotplug.c b/device-hotplug.c
> index fc09d10..e6a1ffb 100644
> --- a/device-hotplug.c
> +++ b/device-hotplug.c
> @@ -76,6 +76,6 @@ void drive_hot_add(Monitor *mon, const QDict *qdict)
>
> err:
> if (dinfo) {
> - drive_put_ref(dinfo);
> + drive_del(dinfo);
> }
> }
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index 40757eb..8651726 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -184,7 +184,7 @@ int pci_piix3_xen_ide_unplug(DeviceState *dev)
> }
> bdrv_close(di->bdrv);
> pci_ide->bus[di->bus].ifs[di->unit].bs = NULL;
> - drive_put_ref(di);
> + drive_del(di);
> }
> }
> qdev_reset_all(DEVICE(dev));
> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> index 0fdbd68..23a5d10 100644
> --- a/include/sysemu/blockdev.h
> +++ b/include/sysemu/blockdev.h
> @@ -43,21 +43,19 @@ struct DriveInfo {
> QemuOpts *opts;
> char *serial;
> QTAILQ_ENTRY(DriveInfo) next;
> - int refcount;
> };
>
> DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
> DriveInfo *drive_get_by_index(BlockInterfaceType type, int index);
> int drive_get_max_bus(BlockInterfaceType type);
> DriveInfo *drive_get_next(BlockInterfaceType type);
> -void drive_get_ref(DriveInfo *dinfo);
> -void drive_put_ref(DriveInfo *dinfo);
> DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
>
> QemuOpts *drive_def(const char *optstr);
> QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
> const char *optstr);
> DriveInfo *drive_new(QemuOpts *arg, BlockInterfaceType block_default_type);
> +void drive_del(DriveInfo *dinfo);
>
> /* device-hotplug */
>
> --
> 1.9.3
>
>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 3/3] block/qapi: Give some functions internal linkage
2014-06-06 12:50 [Qemu-devel] [PATCH 0/3] block: Minor cleanups Markus Armbruster
2014-06-06 12:50 ` [Qemu-devel] [PATCH 1/3] blockdev: Rename drive_init(), drive_uninit() to drive_new(), drive_del() Markus Armbruster
2014-06-06 12:50 ` [Qemu-devel] [PATCH 2/3] blockdev: Remove unused DriveInfo reference count Markus Armbruster
@ 2014-06-06 12:51 ` Markus Armbruster
2014-06-06 14:13 ` Benoît Canet
2014-06-13 13:15 ` Stefan Hajnoczi
2014-06-11 10:36 ` [Qemu-devel] [PATCH 0/3] block: Minor cleanups Stefan Hajnoczi
3 siblings, 2 replies; 10+ messages in thread
From: Markus Armbruster @ 2014-06-06 12:51 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, famz, stefanha
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
block/qapi.c | 13 ++++++-------
include/block/qapi.h | 7 -------
2 files changed, 6 insertions(+), 14 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index 97e1641..f420e9a 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -89,9 +89,9 @@ BlockDeviceInfo *bdrv_block_device_info(BlockDriverState *bs)
* information, or NULL because there are no snapshots. Returns -errno on
* error, with *p_list untouched.
*/
-int bdrv_query_snapshot_info_list(BlockDriverState *bs,
- SnapshotInfoList **p_list,
- Error **errp)
+static int bdrv_query_snapshot_info_list(BlockDriverState *bs,
+ SnapshotInfoList **p_list,
+ Error **errp)
{
int i, sn_count;
QEMUSnapshotInfo *sn_tab = NULL;
@@ -236,9 +236,8 @@ void bdrv_query_image_info(BlockDriverState *bs,
}
/* @p_info will be set only on success. */
-void bdrv_query_info(BlockDriverState *bs,
- BlockInfo **p_info,
- Error **errp)
+static void bdrv_query_info(BlockDriverState *bs, BlockInfo **p_info,
+ Error **errp)
{
BlockInfo *info = g_malloc0(sizeof(*info));
BlockDriverState *bs0;
@@ -293,7 +292,7 @@ void bdrv_query_info(BlockDriverState *bs,
qapi_free_BlockInfo(info);
}
-BlockStats *bdrv_query_stats(const BlockDriverState *bs)
+static BlockStats *bdrv_query_stats(const BlockDriverState *bs)
{
BlockStats *s;
diff --git a/include/block/qapi.h b/include/block/qapi.h
index e92c00d..6672179 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -30,16 +30,9 @@
#include "block/snapshot.h"
BlockDeviceInfo *bdrv_block_device_info(BlockDriverState *bs);
-int bdrv_query_snapshot_info_list(BlockDriverState *bs,
- SnapshotInfoList **p_list,
- Error **errp);
void bdrv_query_image_info(BlockDriverState *bs,
ImageInfo **p_info,
Error **errp);
-void bdrv_query_info(BlockDriverState *bs,
- BlockInfo **p_info,
- Error **errp);
-BlockStats *bdrv_query_stats(const BlockDriverState *bs);
void bdrv_snapshot_dump(fprintf_function func_fprintf, void *f,
QEMUSnapshotInfo *sn);
--
1.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH 3/3] block/qapi: Give some functions internal linkage
2014-06-06 12:51 ` [Qemu-devel] [PATCH 3/3] block/qapi: Give some functions internal linkage Markus Armbruster
@ 2014-06-06 14:13 ` Benoît Canet
2014-06-13 13:15 ` Stefan Hajnoczi
1 sibling, 0 replies; 10+ messages in thread
From: Benoît Canet @ 2014-06-06 14:13 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kwolf, famz, qemu-devel, stefanha
The Friday 06 Jun 2014 à 14:51:00 (+0200), Markus Armbruster wrote :
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> block/qapi.c | 13 ++++++-------
> include/block/qapi.h | 7 -------
> 2 files changed, 6 insertions(+), 14 deletions(-)
>
> diff --git a/block/qapi.c b/block/qapi.c
> index 97e1641..f420e9a 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -89,9 +89,9 @@ BlockDeviceInfo *bdrv_block_device_info(BlockDriverState *bs)
> * information, or NULL because there are no snapshots. Returns -errno on
> * error, with *p_list untouched.
> */
> -int bdrv_query_snapshot_info_list(BlockDriverState *bs,
> - SnapshotInfoList **p_list,
> - Error **errp)
> +static int bdrv_query_snapshot_info_list(BlockDriverState *bs,
> + SnapshotInfoList **p_list,
> + Error **errp)
> {
> int i, sn_count;
> QEMUSnapshotInfo *sn_tab = NULL;
> @@ -236,9 +236,8 @@ void bdrv_query_image_info(BlockDriverState *bs,
> }
>
> /* @p_info will be set only on success. */
> -void bdrv_query_info(BlockDriverState *bs,
> - BlockInfo **p_info,
> - Error **errp)
> +static void bdrv_query_info(BlockDriverState *bs, BlockInfo **p_info,
> + Error **errp)
> {
> BlockInfo *info = g_malloc0(sizeof(*info));
> BlockDriverState *bs0;
> @@ -293,7 +292,7 @@ void bdrv_query_info(BlockDriverState *bs,
> qapi_free_BlockInfo(info);
> }
>
> -BlockStats *bdrv_query_stats(const BlockDriverState *bs)
> +static BlockStats *bdrv_query_stats(const BlockDriverState *bs)
> {
> BlockStats *s;
>
> diff --git a/include/block/qapi.h b/include/block/qapi.h
> index e92c00d..6672179 100644
> --- a/include/block/qapi.h
> +++ b/include/block/qapi.h
> @@ -30,16 +30,9 @@
> #include "block/snapshot.h"
>
> BlockDeviceInfo *bdrv_block_device_info(BlockDriverState *bs);
> -int bdrv_query_snapshot_info_list(BlockDriverState *bs,
> - SnapshotInfoList **p_list,
> - Error **errp);
> void bdrv_query_image_info(BlockDriverState *bs,
> ImageInfo **p_info,
> Error **errp);
> -void bdrv_query_info(BlockDriverState *bs,
> - BlockInfo **p_info,
> - Error **errp);
> -BlockStats *bdrv_query_stats(const BlockDriverState *bs);
>
> void bdrv_snapshot_dump(fprintf_function func_fprintf, void *f,
> QEMUSnapshotInfo *sn);
> --
> 1.9.3
>
>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH 3/3] block/qapi: Give some functions internal linkage
2014-06-06 12:51 ` [Qemu-devel] [PATCH 3/3] block/qapi: Give some functions internal linkage Markus Armbruster
2014-06-06 14:13 ` Benoît Canet
@ 2014-06-13 13:15 ` Stefan Hajnoczi
1 sibling, 0 replies; 10+ messages in thread
From: Stefan Hajnoczi @ 2014-06-13 13:15 UTC (permalink / raw)
To: Markus Armbruster
Cc: Kevin Wolf, Hani Benhabiles, Fam Zheng, qemu-devel,
Stefan Hajnoczi
On Fri, Jun 6, 2014 at 8:51 PM, Markus Armbruster <armbru@redhat.com> wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> block/qapi.c | 13 ++++++-------
> include/block/qapi.h | 7 -------
> 2 files changed, 6 insertions(+), 14 deletions(-)
Unfortunately I had to drop this patch because it conflicts with the
following qemu.git commit which was recently merged:
commit b21631f3b5cdecd14114034cd9e4e7f63ed7fde1
Author: Hani Benhabiles <kroosec@gmail.com>
Date: Tue May 27 23:39:37 2014 +0100
monitor: Add delvm and loadvm argument completion
Signed-off-by: Hani Benhabiles <hani@linux.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
If you want to send an updated patch, please rebase qemu.git. I have
kept the other patches in this series.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] block: Minor cleanups
2014-06-06 12:50 [Qemu-devel] [PATCH 0/3] block: Minor cleanups Markus Armbruster
` (2 preceding siblings ...)
2014-06-06 12:51 ` [Qemu-devel] [PATCH 3/3] block/qapi: Give some functions internal linkage Markus Armbruster
@ 2014-06-11 10:36 ` Stefan Hajnoczi
3 siblings, 0 replies; 10+ messages in thread
From: Stefan Hajnoczi @ 2014-06-11 10:36 UTC (permalink / raw)
To: Markus Armbruster; +Cc: kwolf, famz, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 823 bytes --]
On Fri, Jun 06, 2014 at 02:50:57PM +0200, Markus Armbruster wrote:
> Markus Armbruster (3):
> blockdev: Rename drive_init(), drive_uninit() to drive_new(),
> drive_del()
> blockdev: Remove unused DriveInfo reference count
> block/qapi: Give some functions internal linkage
>
> block/qapi.c | 13 ++++++-------
> blockdev.c | 28 +++++++---------------------
> device-hotplug.c | 4 ++--
> hw/ide/piix.c | 2 +-
> hw/usb/dev-storage.c | 2 +-
> include/block/qapi.h | 7 -------
> include/sysemu/blockdev.h | 8 +++-----
> vl.c | 4 ++--
> 8 files changed, 22 insertions(+), 46 deletions(-)
>
> --
> 1.9.3
>
Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block
Stefan
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread