| Message ID | 20201102094422.173975-10-armbru@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | sockets: Attempt to drain the abstract socket swamp | expand |
On 11/2/20 3:44 AM, Markus Armbruster wrote: > Commit 776b97d360 "qemu-sockets: add abstract UNIX domain socket > support" neglected to update qemu_chr_socket_address(). It shows > shows neither @abstract nor @tight. Fix that. > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > chardev/char-socket.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/chardev/char-socket.c b/chardev/char-socket.c > index 1ee5a8c295..dc1cf86ecf 100644 > --- a/chardev/char-socket.c > +++ b/chardev/char-socket.c > @@ -443,10 +443,18 @@ static char *qemu_chr_socket_address(SocketChardev *s, const char *prefix) > s->is_listen ? ",server" : ""); > break; > case SOCKET_ADDRESS_TYPE_UNIX: > - return g_strdup_printf("%sunix:%s%s", prefix, > + { > + UnixSocketAddress *sa = &s->addr->u.q_unix; > + > + return g_strdup_printf("%sunix:%s%s%s%s", prefix, > s->addr->u.q_unix.path, > + sa->has_abstract && sa->abstract > + ? ",abstract" : "", > + sa->has_tight && sa->tight > + ? ",tight" : "", > s->is_listen ? ",server" : ""); Gets modified again in 11/11, so I can accept this as a strict improvement, even if it is not the final form. Reviewed-by: Eric Blake <eblake@redhat.com> > break; > + } > case SOCKET_ADDRESS_TYPE_FD: > return g_strdup_printf("%sfd:%s%s", prefix, s->addr->u.fd.str, > s->is_listen ? ",server" : ""); > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Eric Blake <eblake@redhat.com> writes: > On 11/2/20 3:44 AM, Markus Armbruster wrote: >> Commit 776b97d360 "qemu-sockets: add abstract UNIX domain socket >> support" neglected to update qemu_chr_socket_address(). It shows >> shows neither @abstract nor @tight. Fix that. >> >> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> chardev/char-socket.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/chardev/char-socket.c b/chardev/char-socket.c >> index 1ee5a8c295..dc1cf86ecf 100644 >> --- a/chardev/char-socket.c >> +++ b/chardev/char-socket.c >> @@ -443,10 +443,18 @@ static char *qemu_chr_socket_address(SocketChardev *s, const char *prefix) >> s->is_listen ? ",server" : ""); >> break; >> case SOCKET_ADDRESS_TYPE_UNIX: >> - return g_strdup_printf("%sunix:%s%s", prefix, >> + { >> + UnixSocketAddress *sa = &s->addr->u.q_unix; >> + >> + return g_strdup_printf("%sunix:%s%s%s%s", prefix, >> s->addr->u.q_unix.path, >> + sa->has_abstract && sa->abstract >> + ? ",abstract" : "", >> + sa->has_tight && sa->tight >> + ? ",tight" : "", >> s->is_listen ? ",server" : ""); > > Gets modified again in 11/11, so I can accept this as a strict > improvement, even if it is not the final form. You're right, PATCH 11's change is better done here already. Will tidy up if I need to respin for some other reason. > Reviewed-by: Eric Blake <eblake@redhat.com> Thanks! > >> break; >> + } >> case SOCKET_ADDRESS_TYPE_FD: >> return g_strdup_printf("%sfd:%s%s", prefix, s->addr->u.fd.str, >> s->is_listen ? ",server" : ""); >>
On Tue, Nov 03, 2020 at 07:28:20AM +0100, Markus Armbruster wrote: > Eric Blake <eblake@redhat.com> writes: > > > On 11/2/20 3:44 AM, Markus Armbruster wrote: > >> Commit 776b97d360 "qemu-sockets: add abstract UNIX domain socket > >> support" neglected to update qemu_chr_socket_address(). It shows > >> shows neither @abstract nor @tight. Fix that. > >> > >> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > >> Signed-off-by: Markus Armbruster <armbru@redhat.com> > >> --- > >> chardev/char-socket.c | 10 +++++++++- > >> 1 file changed, 9 insertions(+), 1 deletion(-) > >> > >> diff --git a/chardev/char-socket.c b/chardev/char-socket.c > >> index 1ee5a8c295..dc1cf86ecf 100644 > >> --- a/chardev/char-socket.c > >> +++ b/chardev/char-socket.c > >> @@ -443,10 +443,18 @@ static char *qemu_chr_socket_address(SocketChardev *s, const char *prefix) > >> s->is_listen ? ",server" : ""); > >> break; > >> case SOCKET_ADDRESS_TYPE_UNIX: > >> - return g_strdup_printf("%sunix:%s%s", prefix, > >> + { > >> + UnixSocketAddress *sa = &s->addr->u.q_unix; > >> + > >> + return g_strdup_printf("%sunix:%s%s%s%s", prefix, > >> s->addr->u.q_unix.path, > >> + sa->has_abstract && sa->abstract > >> + ? ",abstract" : "", > >> + sa->has_tight && sa->tight > >> + ? ",tight" : "", > >> s->is_listen ? ",server" : ""); > > > > Gets modified again in 11/11, so I can accept this as a strict > > improvement, even if it is not the final form. > > You're right, PATCH 11's change is better done here already. Will tidy > up if I need to respin for some other reason. I can squash in the following part of patch 11: @@ -444,14 +444,20 @@ static char *qemu_chr_socket_address(SocketChardev *s, const char *prefix) break; case SOCKET_ADDRESS_TYPE_UNIX: { + const char *tight = "", *abstract = ""; UnixSocketAddress *sa = &s->addr->u.q_unix; - return g_strdup_printf("%sunix:%s%s%s%s", prefix, - s->addr->u.q_unix.path, - sa->has_abstract && sa->abstract - ? ",abstract" : "", - sa->has_tight && sa->tight - ? ",tight" : "", +#ifdef CONFIG_LINUX + if (sa->has_abstract && sa->abstract) { + abstract = ",abstract"; + if (sa->has_tight && sa->tight) { + tight = ",tight"; + } + } +#endif + + return g_strdup_printf("%sunix:%s%s%s%s", prefix, sa->path, + abstract, tight, s->is_listen ? ",server" : ""); break; } but leaving out the CONFIG_LINUX ifdef until Patch 11 Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Daniel P. Berrangé <berrange@redhat.com> writes: > On Tue, Nov 03, 2020 at 07:28:20AM +0100, Markus Armbruster wrote: >> Eric Blake <eblake@redhat.com> writes: >> >> > On 11/2/20 3:44 AM, Markus Armbruster wrote: >> >> Commit 776b97d360 "qemu-sockets: add abstract UNIX domain socket >> >> support" neglected to update qemu_chr_socket_address(). It shows >> >> shows neither @abstract nor @tight. Fix that. >> >> >> >> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> >> --- >> >> chardev/char-socket.c | 10 +++++++++- >> >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/chardev/char-socket.c b/chardev/char-socket.c >> >> index 1ee5a8c295..dc1cf86ecf 100644 >> >> --- a/chardev/char-socket.c >> >> +++ b/chardev/char-socket.c >> >> @@ -443,10 +443,18 @@ static char *qemu_chr_socket_address(SocketChardev *s, const char *prefix) >> >> s->is_listen ? ",server" : ""); >> >> break; >> >> case SOCKET_ADDRESS_TYPE_UNIX: >> >> - return g_strdup_printf("%sunix:%s%s", prefix, >> >> + { >> >> + UnixSocketAddress *sa = &s->addr->u.q_unix; >> >> + >> >> + return g_strdup_printf("%sunix:%s%s%s%s", prefix, >> >> s->addr->u.q_unix.path, >> >> + sa->has_abstract && sa->abstract >> >> + ? ",abstract" : "", >> >> + sa->has_tight && sa->tight >> >> + ? ",tight" : "", >> >> s->is_listen ? ",server" : ""); >> > >> > Gets modified again in 11/11, so I can accept this as a strict >> > improvement, even if it is not the final form. >> >> You're right, PATCH 11's change is better done here already. Will tidy >> up if I need to respin for some other reason. > > I can squash in the following part of patch 11: > > @@ -444,14 +444,20 @@ static char *qemu_chr_socket_address(SocketChardev *s, const char *prefix) > break; > case SOCKET_ADDRESS_TYPE_UNIX: > { > + const char *tight = "", *abstract = ""; > UnixSocketAddress *sa = &s->addr->u.q_unix; > > - return g_strdup_printf("%sunix:%s%s%s%s", prefix, > - s->addr->u.q_unix.path, > - sa->has_abstract && sa->abstract > - ? ",abstract" : "", > - sa->has_tight && sa->tight > - ? ",tight" : "", > +#ifdef CONFIG_LINUX > + if (sa->has_abstract && sa->abstract) { > + abstract = ",abstract"; > + if (sa->has_tight && sa->tight) { > + tight = ",tight"; > + } > + } > +#endif > + > + return g_strdup_printf("%sunix:%s%s%s%s", prefix, sa->path, > + abstract, tight, > s->is_listen ? ",server" : ""); > break; > } > > but leaving out the CONFIG_LINUX ifdef until Patch 11 Appreciated!
diff --git a/chardev/char-socket.c b/chardev/char-socket.c index 1ee5a8c295..dc1cf86ecf 100644 --- a/chardev/char-socket.c +++ b/chardev/char-socket.c @@ -443,10 +443,18 @@ static char *qemu_chr_socket_address(SocketChardev *s, const char *prefix) s->is_listen ? ",server" : ""); break; case SOCKET_ADDRESS_TYPE_UNIX: - return g_strdup_printf("%sunix:%s%s", prefix, + { + UnixSocketAddress *sa = &s->addr->u.q_unix; + + return g_strdup_printf("%sunix:%s%s%s%s", prefix, s->addr->u.q_unix.path, + sa->has_abstract && sa->abstract + ? ",abstract" : "", + sa->has_tight && sa->tight + ? ",tight" : "", s->is_listen ? ",server" : ""); break; + } case SOCKET_ADDRESS_TYPE_FD: return g_strdup_printf("%sfd:%s%s", prefix, s->addr->u.fd.str, s->is_listen ? ",server" : "");