* [Qemu-devel] [PATCH] char: Allow devices to use a single multiplexed chardev.
@ 2011-04-22 12:59 Kusanagi Kouichi
2011-04-25 9:57 ` Amit Shah
0 siblings, 1 reply; 5+ messages in thread
From: Kusanagi Kouichi @ 2011-04-22 12:59 UTC (permalink / raw)
To: qemu-devel
This fixes regression caused by commit
2d6c1ef40f3678ab47a4d14fb5dadaa486bfcda6
("char: Prevent multiple devices opening same chardev").
Signed-off-by: Kusanagi Kouichi <slash@ac.auone-net.jp>
---
hw/qdev-properties.c | 4 ++--
qemu-char.c | 5 ++++-
qemu-char.h | 2 +-
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 1088a26..0eed712 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -354,10 +354,10 @@ static int parse_chr(DeviceState *dev, Property *prop, const char *str)
if (*ptr == NULL) {
return -ENOENT;
}
- if ((*ptr)->assigned) {
+ if ((*ptr)->avail < 1) {
return -EEXIST;
}
- (*ptr)->assigned = 1;
+ --(*ptr)->avail;
return 0;
}
diff --git a/qemu-char.c b/qemu-char.c
index 03858d4..f08f2b8 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -199,7 +199,7 @@ void qemu_chr_add_handlers(CharDriverState *s,
{
if (!opaque) {
/* chr driver being released. */
- s->assigned = 0;
+ ++s->avail;
}
s->chr_can_read = fd_can_read;
s->chr_read = fd_read;
@@ -2544,7 +2544,10 @@ CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
snprintf(base->label, len, "%s-base", qemu_opts_id(opts));
chr = qemu_chr_open_mux(base);
chr->filename = base->filename;
+ chr->avail = MAX_MUX;
QTAILQ_INSERT_TAIL(&chardevs, chr, next);
+ } else {
+ chr->avail = 1;
}
chr->label = qemu_strdup(qemu_opts_id(opts));
return chr;
diff --git a/qemu-char.h b/qemu-char.h
index fb96eef..ebf3641 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -70,7 +70,7 @@ struct CharDriverState {
char *label;
char *filename;
int opened;
- int assigned; /* chardev assigned to a device */
+ int avail;
QTAILQ_ENTRY(CharDriverState) next;
};
--
1.7.4.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] char: Allow devices to use a single multiplexed chardev.
2011-04-22 12:59 [Qemu-devel] [PATCH] char: Allow devices to use a single multiplexed chardev Kusanagi Kouichi
@ 2011-04-25 9:57 ` Amit Shah
2011-04-25 15:30 ` Kusanagi Kouichi
0 siblings, 1 reply; 5+ messages in thread
From: Amit Shah @ 2011-04-25 9:57 UTC (permalink / raw)
To: Kusanagi Kouichi; +Cc: qemu-devel
On (Fri) 22 Apr 2011 [21:59:42], Kusanagi Kouichi wrote:
> This fixes regression caused by commit
> 2d6c1ef40f3678ab47a4d14fb5dadaa486bfcda6
> ("char: Prevent multiple devices opening same chardev").
What's the regression? How do I test it?
> Signed-off-by: Kusanagi Kouichi <slash@ac.auone-net.jp>
> ---
> hw/qdev-properties.c | 4 ++--
> qemu-char.c | 5 ++++-
> qemu-char.h | 2 +-
> 3 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index 1088a26..0eed712 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -354,10 +354,10 @@ static int parse_chr(DeviceState *dev, Property *prop, const char *str)
> if (*ptr == NULL) {
> return -ENOENT;
> }
> - if ((*ptr)->assigned) {
> + if ((*ptr)->avail < 1) {
> return -EEXIST;
> }
> - (*ptr)->assigned = 1;
> + --(*ptr)->avail;
> return 0;
> }
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 03858d4..f08f2b8 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -199,7 +199,7 @@ void qemu_chr_add_handlers(CharDriverState *s,
> {
> if (!opaque) {
> /* chr driver being released. */
> - s->assigned = 0;
> + ++s->avail;
> }
Will just checking for handlers (fd_can_read, fd_read, fd_write not
NULL) here help instead of this patch?
> s->chr_can_read = fd_can_read;
> s->chr_read = fd_read;
> @@ -2544,7 +2544,10 @@ CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
> snprintf(base->label, len, "%s-base", qemu_opts_id(opts));
> chr = qemu_chr_open_mux(base);
> chr->filename = base->filename;
> + chr->avail = MAX_MUX;
> QTAILQ_INSERT_TAIL(&chardevs, chr, next);
> + } else {
> + chr->avail = 1;
> }
> chr->label = qemu_strdup(qemu_opts_id(opts));
> return chr;
> diff --git a/qemu-char.h b/qemu-char.h
> index fb96eef..ebf3641 100644
> --- a/qemu-char.h
> +++ b/qemu-char.h
> @@ -70,7 +70,7 @@ struct CharDriverState {
> char *label;
> char *filename;
> int opened;
> - int assigned; /* chardev assigned to a device */
> + int avail;
> QTAILQ_ENTRY(CharDriverState) next;
> };
>
> --
> 1.7.4.4
>
>
Amit
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] char: Allow devices to use a single multiplexed chardev.
2011-04-25 9:57 ` Amit Shah
@ 2011-04-25 15:30 ` Kusanagi Kouichi
2011-04-26 4:32 ` Amit Shah
0 siblings, 1 reply; 5+ messages in thread
From: Kusanagi Kouichi @ 2011-04-25 15:30 UTC (permalink / raw)
To: Amit Shah; +Cc: qemu-devel
On 2011-04-25 15:27:20 +0530, Amit Shah wrote:
> On (Fri) 22 Apr 2011 [21:59:42], Kusanagi Kouichi wrote:
> > This fixes regression caused by commit
> > 2d6c1ef40f3678ab47a4d14fb5dadaa486bfcda6
> > ("char: Prevent multiple devices opening same chardev").
>
> What's the regression? How do I test it?
>
-nodefaults -nographic -chardev stdio,id=stdio,mux=on,signal=off -mon stdio -device virtio-serial-pci -device virtconsole,chardev=stdio -device isa-serial,chardev=stdio
fails with
qemu-system-x86_64: -device isa-serial,chardev=stdio: Property 'isa-serial.chardev' can't take value 'stdio', it's in use
Is this intended?
> > Signed-off-by: Kusanagi Kouichi <slash@ac.auone-net.jp>
> > ---
> > hw/qdev-properties.c | 4 ++--
> > qemu-char.c | 5 ++++-
> > qemu-char.h | 2 +-
> > 3 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> > index 1088a26..0eed712 100644
> > --- a/hw/qdev-properties.c
> > +++ b/hw/qdev-properties.c
> > @@ -354,10 +354,10 @@ static int parse_chr(DeviceState *dev, Property *prop, const char *str)
> > if (*ptr == NULL) {
> > return -ENOENT;
> > }
> > - if ((*ptr)->assigned) {
> > + if ((*ptr)->avail < 1) {
> > return -EEXIST;
> > }
> > - (*ptr)->assigned = 1;
> > + --(*ptr)->avail;
> > return 0;
> > }
> >
> > diff --git a/qemu-char.c b/qemu-char.c
> > index 03858d4..f08f2b8 100644
> > --- a/qemu-char.c
> > +++ b/qemu-char.c
> > @@ -199,7 +199,7 @@ void qemu_chr_add_handlers(CharDriverState *s,
> > {
> > if (!opaque) {
> > /* chr driver being released. */
> > - s->assigned = 0;
> > + ++s->avail;
> > }
>
> Will just checking for handlers (fd_can_read, fd_read, fd_write not
> NULL) here help instead of this patch?
>
That doesn't help.
> > s->chr_can_read = fd_can_read;
> > s->chr_read = fd_read;
> > @@ -2544,7 +2544,10 @@ CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
> > snprintf(base->label, len, "%s-base", qemu_opts_id(opts));
> > chr = qemu_chr_open_mux(base);
> > chr->filename = base->filename;
> > + chr->avail = MAX_MUX;
> > QTAILQ_INSERT_TAIL(&chardevs, chr, next);
> > + } else {
> > + chr->avail = 1;
> > }
> > chr->label = qemu_strdup(qemu_opts_id(opts));
> > return chr;
> > diff --git a/qemu-char.h b/qemu-char.h
> > index fb96eef..ebf3641 100644
> > --- a/qemu-char.h
> > +++ b/qemu-char.h
> > @@ -70,7 +70,7 @@ struct CharDriverState {
> > char *label;
> > char *filename;
> > int opened;
> > - int assigned; /* chardev assigned to a device */
> > + int avail;
> > QTAILQ_ENTRY(CharDriverState) next;
> > };
> >
> > --
> > 1.7.4.4
> >
> >
>
> Amit
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] char: Allow devices to use a single multiplexed chardev.
2011-04-25 15:30 ` Kusanagi Kouichi
@ 2011-04-26 4:32 ` Amit Shah
0 siblings, 0 replies; 5+ messages in thread
From: Amit Shah @ 2011-04-26 4:32 UTC (permalink / raw)
To: Kusanagi Kouichi; +Cc: qemu-devel
On (Tue) 26 Apr 2011 [00:30:28], Kusanagi Kouichi wrote:
> On 2011-04-25 15:27:20 +0530, Amit Shah wrote:
> > On (Fri) 22 Apr 2011 [21:59:42], Kusanagi Kouichi wrote:
> > > This fixes regression caused by commit
> > > 2d6c1ef40f3678ab47a4d14fb5dadaa486bfcda6
> > > ("char: Prevent multiple devices opening same chardev").
> >
> > What's the regression? How do I test it?
> >
>
> -nodefaults -nographic -chardev stdio,id=stdio,mux=on,signal=off -mon stdio -device virtio-serial-pci -device virtconsole,chardev=stdio -device isa-serial,chardev=stdio
> fails with
> qemu-system-x86_64: -device isa-serial,chardev=stdio: Property 'isa-serial.chardev' can't take value 'stdio', it's in use
OK, so please mention it in the commit message :-)
> Is this intended?
No, it's not.
Just one more thing:
> > > diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> > > index 1088a26..0eed712 100644
> > > --- a/hw/qdev-properties.c
> > > +++ b/hw/qdev-properties.c
> > > @@ -354,10 +354,10 @@ static int parse_chr(DeviceState *dev, Property *prop, const char *str)
> > > if (*ptr == NULL) {
> > > return -ENOENT;
> > > }
> > > - if ((*ptr)->assigned) {
> > > + if ((*ptr)->avail < 1) {
> > > return -EEXIST;
> > > }
> > > - (*ptr)->assigned = 1;
> > > + --(*ptr)->avail;
> > > return 0;
> > > }
'avail' isn't readily intuitive. Can you use a better name, like
'avail_connections' or something like that?
Please CC me on the updated patch.
Amit
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH] char: Allow devices to use a single multiplexed chardev.
@ 2011-04-26 10:19 Kusanagi Kouichi
0 siblings, 0 replies; 5+ messages in thread
From: Kusanagi Kouichi @ 2011-04-26 10:19 UTC (permalink / raw)
To: qemu-devel; +Cc: Amit Shah
This fixes regression caused by commit
2d6c1ef40f3678ab47a4d14fb5dadaa486bfcda6
("char: Prevent multiple devices opening same chardev").
-nodefaults -nographic -chardev stdio,id=stdio,mux=on,signal=off -mon stdio -device virtio-serial-pci -device virtconsole,chardev=stdio -device isa-serial,chardev=stdio
fails with
qemu-system-x86_64: -device isa-serial,chardev=stdio: Property 'isa-serial.chardev' can't take value 'stdio', it's in use
Signed-off-by: Kusanagi Kouichi <slash@ac.auone-net.jp>
---
hw/qdev-properties.c | 4 ++--
qemu-char.c | 5 ++++-
qemu-char.h | 2 +-
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 1088a26..eff2d24 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -354,10 +354,10 @@ static int parse_chr(DeviceState *dev, Property *prop, const char *str)
if (*ptr == NULL) {
return -ENOENT;
}
- if ((*ptr)->assigned) {
+ if ((*ptr)->avail_connections < 1) {
return -EEXIST;
}
- (*ptr)->assigned = 1;
+ --(*ptr)->avail_connections;
return 0;
}
diff --git a/qemu-char.c b/qemu-char.c
index 03858d4..f5c2dbc 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -199,7 +199,7 @@ void qemu_chr_add_handlers(CharDriverState *s,
{
if (!opaque) {
/* chr driver being released. */
- s->assigned = 0;
+ ++s->avail_connections;
}
s->chr_can_read = fd_can_read;
s->chr_read = fd_read;
@@ -2544,7 +2544,10 @@ CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
snprintf(base->label, len, "%s-base", qemu_opts_id(opts));
chr = qemu_chr_open_mux(base);
chr->filename = base->filename;
+ chr->avail_connections = MAX_MUX;
QTAILQ_INSERT_TAIL(&chardevs, chr, next);
+ } else {
+ chr->avail_connections = 1;
}
chr->label = qemu_strdup(qemu_opts_id(opts));
return chr;
diff --git a/qemu-char.h b/qemu-char.h
index fb96eef..f669827 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -70,7 +70,7 @@ struct CharDriverState {
char *label;
char *filename;
int opened;
- int assigned; /* chardev assigned to a device */
+ int avail_connections;
QTAILQ_ENTRY(CharDriverState) next;
};
--
1.7.4.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-04-26 10:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-04-22 12:59 [Qemu-devel] [PATCH] char: Allow devices to use a single multiplexed chardev Kusanagi Kouichi
2011-04-25 9:57 ` Amit Shah
2011-04-25 15:30 ` Kusanagi Kouichi
2011-04-26 4:32 ` Amit Shah
-- strict thread matches above, loose matches on Subject: below --
2011-04-26 10:19 Kusanagi Kouichi
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).