* [Qemu-devel] [PATCH] chardev: Get filename for new qapi backend
@ 2013-05-15 3:52 Lei Li
2013-05-15 5:39 ` Gerd Hoffmann
2013-05-22 22:59 ` Anthony Liguori
0 siblings, 2 replies; 6+ messages in thread
From: Lei Li @ 2013-05-15 3:52 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, kraxel, Lei Li
This patch add the filename when the new qapi backend init from opts.
Commit 2c5f488293c7d0cd095635c74157c2526e2c4947 add support for
qapi-based chardev initialization, but miss the filename of the
char device as below:
(qemu) info chardev
parallel0: filename=(null)
serial0: filename=(null)
compat_monitor0: filename=(null)
Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
qemu-char.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 30a2ddf..a9f618a 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3276,6 +3276,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
ChardevReturn *ret = NULL;
const char *id = qemu_opts_id(opts);
const char *bid = NULL;
+ char *filename = g_strdup(qemu_opt_get(opts, "backend"));
if (qemu_opt_get_bool(opts, "mux", 0)) {
bid = g_strdup_printf("%s-base", id);
@@ -3308,6 +3309,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
}
chr = qemu_chr_find(id);
+ chr->filename = filename;
qapi_out:
qapi_free_ChardevBackend(backend);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] chardev: Get filename for new qapi backend
2013-05-15 3:52 [Qemu-devel] [PATCH] chardev: Get filename for new qapi backend Lei Li
@ 2013-05-15 5:39 ` Gerd Hoffmann
2013-05-15 7:03 ` Lei Li
2013-05-15 7:26 ` Lei Li
2013-05-22 22:59 ` Anthony Liguori
1 sibling, 2 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2013-05-15 5:39 UTC (permalink / raw)
To: Lei Li; +Cc: aliguori, qemu-devel
On 05/15/13 05:52, Lei Li wrote:
> This patch add the filename when the new qapi backend init from opts.
>
> Commit 2c5f488293c7d0cd095635c74157c2526e2c4947 add support for
> qapi-based chardev initialization, but miss the filename of the
> char device as below:
>
> (qemu) info chardev
> parallel0: filename=(null)
> serial0: filename=(null)
> compat_monitor0: filename=(null)
> @@ -3276,6 +3276,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
> ChardevReturn *ret = NULL;
> const char *id = qemu_opts_id(opts);
> const char *bid = NULL;
> + char *filename = g_strdup(qemu_opt_get(opts, "backend"));
>
> if (qemu_opt_get_bool(opts, "mux", 0)) {
> bid = g_strdup_printf("%s-base", id);
> @@ -3308,6 +3309,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
> }
>
> chr = qemu_chr_find(id);
> + chr->filename = filename;
That should happen in qmp_chardev_add(), so filename is set consistently
for every chardev no matter how it gets created.
Take care to not overwrite filename, some functions
(qmp_chardev_open_socket for example) do set chr->filename already.
Maybe it's better to put that into the individual qmp_chardev_open_*
functions anyway.
cheers,
Gerd
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] chardev: Get filename for new qapi backend
2013-05-15 5:39 ` Gerd Hoffmann
@ 2013-05-15 7:03 ` Lei Li
2013-05-15 7:26 ` Lei Li
1 sibling, 0 replies; 6+ messages in thread
From: Lei Li @ 2013-05-15 7:03 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: aliguori, qemu-devel
On 05/15/2013 01:39 PM, Gerd Hoffmann wrote:
> On 05/15/13 05:52, Lei Li wrote:
>> This patch add the filename when the new qapi backend init from opts.
>>
>> Commit 2c5f488293c7d0cd095635c74157c2526e2c4947 add support for
>> qapi-based chardev initialization, but miss the filename of the
>> char device as below:
>>
>> (qemu) info chardev
>> parallel0: filename=(null)
>> serial0: filename=(null)
>> compat_monitor0: filename=(null)
>> @@ -3276,6 +3276,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
>> ChardevReturn *ret = NULL;
>> const char *id = qemu_opts_id(opts);
>> const char *bid = NULL;
>> + char *filename = g_strdup(qemu_opt_get(opts, "backend"));
>>
>> if (qemu_opt_get_bool(opts, "mux", 0)) {
>> bid = g_strdup_printf("%s-base", id);
>> @@ -3308,6 +3309,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
>> }
>>
>> chr = qemu_chr_find(id);
>> + chr->filename = filename;
> That should happen in qmp_chardev_add(), so filename is set consistently
> for every chardev no matter how it gets created.
Yeah, I was hesitant to add it here for that it does not looks very logical/...
/But I am not sure about the best place for it..
// <http://www.google.com.hk/search?newwindow=1&safe=strict&biw=1280&bih=646&q=i+am+a+little+Hesitant&spell=1&sa=X&ei=OiaTUaGALpPI4AOOoIDoCQ&ved=0CCkQvwUoAA>
>
> Take care to not overwrite filename, some functions
> (qmp_chardev_open_socket for example) do set chr->filename already.
Yes, for the char device pipe, pty, socket, they set chr->filename in their
own open functions. But I think the overwrite will not happen because
it will not get "backend" by qemu_opt_get() for these chardevs.
>
> Maybe it's better to put that into the individual qmp_chardev_open_*
> functions anyway.
OK, I'll have a try and see if everyone like it, I don't have better idea..
>
> cheers,
> Gerd
>
>
--
Lei
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] chardev: Get filename for new qapi backend
2013-05-15 5:39 ` Gerd Hoffmann
2013-05-15 7:03 ` Lei Li
@ 2013-05-15 7:26 ` Lei Li
2013-05-17 8:35 ` Lei Li
1 sibling, 1 reply; 6+ messages in thread
From: Lei Li @ 2013-05-15 7:26 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: aliguori, qemu-devel
On 05/15/2013 01:39 PM, Gerd Hoffmann wrote:
> On 05/15/13 05:52, Lei Li wrote:
>> This patch add the filename when the new qapi backend init from opts.
>>
>> Commit 2c5f488293c7d0cd095635c74157c2526e2c4947 add support for
>> qapi-based chardev initialization, but miss the filename of the
>> char device as below:
>>
>> (qemu) info chardev
>> parallel0: filename=(null)
>> serial0: filename=(null)
>> compat_monitor0: filename=(null)
>> @@ -3276,6 +3276,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
>> ChardevReturn *ret = NULL;
>> const char *id = qemu_opts_id(opts);
>> const char *bid = NULL;
>> + char *filename = g_strdup(qemu_opt_get(opts, "backend"));
>>
>> if (qemu_opt_get_bool(opts, "mux", 0)) {
>> bid = g_strdup_printf("%s-base", id);
>> @@ -3308,6 +3309,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
>> }
>>
>> chr = qemu_chr_find(id);
>> + chr->filename = filename;
> That should happen in qmp_chardev_add(), so filename is set consistently
> for every chardev no matter how it gets created.
Yeah, I was hesitant to add it here for that it does not looks very logical...
But I am not sure about the best place for it..
> Take care to not overwrite filename, some functions
> (qmp_chardev_open_socket for example) do set chr->filename already.
Yes, for the char device pipe, pty, socket, they set chr->filename in their
own open functions. But I think the overwrite will not happen because
it will not get "backend" by qemu_opt_get() for these chardevs.
>
> Maybe it's better to put that into the individual qmp_chardev_open_*
> functions anyway.
OK, I'll have a try and see if everyone like it, I don't have better idea.
>
> cheers,
> Gerd
>
>
>
--
Lei
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] chardev: Get filename for new qapi backend
2013-05-15 7:26 ` Lei Li
@ 2013-05-17 8:35 ` Lei Li
0 siblings, 0 replies; 6+ messages in thread
From: Lei Li @ 2013-05-17 8:35 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: aliguori, qemu-devel
On 05/15/2013 03:26 PM, Lei Li wrote:
> On 05/15/2013 01:39 PM, Gerd Hoffmann wrote:
>> On 05/15/13 05:52, Lei Li wrote:
>>> This patch add the filename when the new qapi backend init from opts.
>>>
>>> Commit 2c5f488293c7d0cd095635c74157c2526e2c4947 add support for
>>> qapi-based chardev initialization, but miss the filename of the
>>> char device as below:
>>>
>>> (qemu) info chardev
>>> parallel0: filename=(null)
>>> serial0: filename=(null)
>>> compat_monitor0: filename=(null)
>>> @@ -3276,6 +3276,7 @@ CharDriverState
>>> *qemu_chr_new_from_opts(QemuOpts *opts,
>>> ChardevReturn *ret = NULL;
>>> const char *id = qemu_opts_id(opts);
>>> const char *bid = NULL;
>>> + char *filename = g_strdup(qemu_opt_get(opts, "backend"));
>>> if (qemu_opt_get_bool(opts, "mux", 0)) {
>>> bid = g_strdup_printf("%s-base", id);
>>> @@ -3308,6 +3309,7 @@ CharDriverState
>>> *qemu_chr_new_from_opts(QemuOpts *opts,
>>> }
>>> chr = qemu_chr_find(id);
>>> + chr->filename = filename;
>> That should happen in qmp_chardev_add(), so filename is set consistently
>> for every chardev no matter how it gets created.
>
> Yeah, I was hesitant to add it here for that it does not looks very
> logical...
> But I am not sure about the best place for it..
>
>> Take care to not overwrite filename, some functions
>> (qmp_chardev_open_socket for example) do set chr->filename already.
>
> Yes, for the char device pipe, pty, socket, they set chr->filename in
> their
> own open functions. But I think the overwrite will not happen because
> it will not get "backend" by qemu_opt_get() for these chardevs.
>
>>
>> Maybe it's better to put that into the individual qmp_chardev_open_*
>> functions anyway.
>
> OK, I'll have a try and see if everyone like it, I don't have better
> idea.
>
Hi Gerd,
As I tried to add the filename into individual open functions, I think it might
not be a good idea. For example,
chardev backend file and console have the same open function qemu_chr_open_win_file().
Then we may need to add flag to distinguish them.
And we already parse every backend into QemuOpts, if we want to put the filename setting
job into open functions (by hardcoding) seems a little overdone?
So why not just set it in qemu_chr_new_from_opts(), by qemu_opt_get() directly?
>>
>> cheers,
>> Gerd
>>
>>
>>
>
>
--
Lei
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] chardev: Get filename for new qapi backend
2013-05-15 3:52 [Qemu-devel] [PATCH] chardev: Get filename for new qapi backend Lei Li
2013-05-15 5:39 ` Gerd Hoffmann
@ 2013-05-22 22:59 ` Anthony Liguori
1 sibling, 0 replies; 6+ messages in thread
From: Anthony Liguori @ 2013-05-22 22:59 UTC (permalink / raw)
To: Lei Li, qemu-devel; +Cc: aliguori
Applied. Thanks.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-05-22 22:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-15 3:52 [Qemu-devel] [PATCH] chardev: Get filename for new qapi backend Lei Li
2013-05-15 5:39 ` Gerd Hoffmann
2013-05-15 7:03 ` Lei Li
2013-05-15 7:26 ` Lei Li
2013-05-17 8:35 ` Lei Li
2013-05-22 22:59 ` Anthony Liguori
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).