| Message ID | 20220709101035.2989428-1-dmitry.semenets@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/4] tools: remove xenstore entries on vchan server closure | expand |
On 09.07.22 12:10, dmitry.semenets@gmail.com wrote: > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > vchan server creates XenStore entries to advertise its event channel and > ring, but those are not removed after the server quits. > Add additional cleanup step, so those are removed, so clients do not try > to connect to a non-existing server. > > Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > --- > tools/include/libxenvchan.h | 5 +++++ > tools/libs/vchan/init.c | 23 +++++++++++++++++++++++ > tools/libs/vchan/io.c | 4 ++++ > tools/libs/vchan/vchan.h | 31 +++++++++++++++++++++++++++++++ > 4 files changed, 63 insertions(+) > create mode 100644 tools/libs/vchan/vchan.h > > diff --git a/tools/include/libxenvchan.h b/tools/include/libxenvchan.h > index d6010b145d..30cc73cf97 100644 > --- a/tools/include/libxenvchan.h > +++ b/tools/include/libxenvchan.h > @@ -86,6 +86,11 @@ struct libxenvchan { > int blocking:1; > /* communication rings */ > struct libxenvchan_ring read, write; > + /** > + * Base xenstore path for storing ring/event data used by the server > + * during cleanup. > + * */ > + char *xs_path; > }; > > /** > diff --git a/tools/libs/vchan/init.c b/tools/libs/vchan/init.c > index c8510e6ce9..c6b8674ef5 100644 > --- a/tools/libs/vchan/init.c > +++ b/tools/libs/vchan/init.c > @@ -46,6 +46,8 @@ > #include <xen/sys/gntdev.h> > #include <libxenvchan.h> > > +#include "vchan.h" > + > #ifndef PAGE_SHIFT > #define PAGE_SHIFT 12 > #endif > @@ -251,6 +253,10 @@ static int init_xs_srv(struct libxenvchan *ctrl, int domain, const char* xs_base > char ref[16]; > char* domid_str = NULL; > xs_transaction_t xs_trans = XBT_NULL; > + > + // store the base path so we can clean up on server closure Please don't introduce new usages of the C++ comment style. > + ctrl->xs_path = strdup(xs_base); Return an error in case of strdup() failure? > + > xs = xs_open(0); > if (!xs) > goto fail; > @@ -298,6 +304,23 @@ retry_transaction: > return ret; > } > > +void close_xs_srv(struct libxenvchan *ctrl) > +{ > + struct xs_handle *xs; > + > + if (!ctrl->xs_path) > + return; > + > + xs = xs_open(0); > + if (!xs) > + goto fail; > + > + xs_rm(xs, XBT_NULL, ctrl->xs_path); In this simple case I'd prefer if (xs) xs_rm(xs, XBT_NULL, ctrl->xs_path); > + > +fail: > + free(ctrl->xs_path); No xs_close()? Juergen
пн, 11 июл. 2022 г. в 10:09, Juergen Gross <jgross@suse.com>: > > On 09.07.22 12:10, dmitry.semenets@gmail.com wrote: > > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > > > vchan server creates XenStore entries to advertise its event channel and > > ring, but those are not removed after the server quits. > > Add additional cleanup step, so those are removed, so clients do not try > > to connect to a non-existing server. > > > > Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > --- > > tools/include/libxenvchan.h | 5 +++++ > > tools/libs/vchan/init.c | 23 +++++++++++++++++++++++ > > tools/libs/vchan/io.c | 4 ++++ > > tools/libs/vchan/vchan.h | 31 +++++++++++++++++++++++++++++++ > > 4 files changed, 63 insertions(+) > > create mode 100644 tools/libs/vchan/vchan.h > > > > diff --git a/tools/include/libxenvchan.h b/tools/include/libxenvchan.h > > index d6010b145d..30cc73cf97 100644 > > --- a/tools/include/libxenvchan.h > > +++ b/tools/include/libxenvchan.h > > @@ -86,6 +86,11 @@ struct libxenvchan { > > int blocking:1; > > /* communication rings */ > > struct libxenvchan_ring read, write; > > + /** > > + * Base xenstore path for storing ring/event data used by the server > > + * during cleanup. > > + * */ > > + char *xs_path; > > }; > > > > /** > > diff --git a/tools/libs/vchan/init.c b/tools/libs/vchan/init.c > > index c8510e6ce9..c6b8674ef5 100644 > > --- a/tools/libs/vchan/init.c > > +++ b/tools/libs/vchan/init.c > > @@ -46,6 +46,8 @@ > > #include <xen/sys/gntdev.h> > > #include <libxenvchan.h> > > > > +#include "vchan.h" > > + > > #ifndef PAGE_SHIFT > > #define PAGE_SHIFT 12 > > #endif > > @@ -251,6 +253,10 @@ static int init_xs_srv(struct libxenvchan *ctrl, int domain, const char* xs_base > > char ref[16]; > > char* domid_str = NULL; > > xs_transaction_t xs_trans = XBT_NULL; > > + > > + // store the base path so we can clean up on server closure > > Please don't introduce new usages of the C++ comment style. Most comments in this file are C++ style since libvchan introduced > > > + ctrl->xs_path = strdup(xs_base); > > Return an error in case of strdup() failure? > > > + > > xs = xs_open(0); > > if (!xs) > > goto fail; > > @@ -298,6 +304,23 @@ retry_transaction: > > return ret; > > } > > > > +void close_xs_srv(struct libxenvchan *ctrl) > > +{ > > + struct xs_handle *xs; > > + > > + if (!ctrl->xs_path) > > + return; > > + > > + xs = xs_open(0); > > + if (!xs) > > + goto fail; > > + > > + xs_rm(xs, XBT_NULL, ctrl->xs_path); > > In this simple case I'd prefer > > if (xs) > xs_rm(xs, XBT_NULL, ctrl->xs_path); > > > + > > +fail: > > + free(ctrl->xs_path); > > No xs_close()? > > > Juergen
On 11.07.22 14:12, Dmytro Semenets wrote: > пн, 11 июл. 2022 г. в 10:09, Juergen Gross <jgross@suse.com>: >> >> On 09.07.22 12:10, dmitry.semenets@gmail.com wrote: >>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >>> >>> vchan server creates XenStore entries to advertise its event channel and >>> ring, but those are not removed after the server quits. >>> Add additional cleanup step, so those are removed, so clients do not try >>> to connect to a non-existing server. >>> >>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >>> --- >>> tools/include/libxenvchan.h | 5 +++++ >>> tools/libs/vchan/init.c | 23 +++++++++++++++++++++++ >>> tools/libs/vchan/io.c | 4 ++++ >>> tools/libs/vchan/vchan.h | 31 +++++++++++++++++++++++++++++++ >>> 4 files changed, 63 insertions(+) >>> create mode 100644 tools/libs/vchan/vchan.h >>> >>> diff --git a/tools/include/libxenvchan.h b/tools/include/libxenvchan.h >>> index d6010b145d..30cc73cf97 100644 >>> --- a/tools/include/libxenvchan.h >>> +++ b/tools/include/libxenvchan.h >>> @@ -86,6 +86,11 @@ struct libxenvchan { >>> int blocking:1; >>> /* communication rings */ >>> struct libxenvchan_ring read, write; >>> + /** >>> + * Base xenstore path for storing ring/event data used by the server >>> + * during cleanup. >>> + * */ >>> + char *xs_path; >>> }; >>> >>> /** >>> diff --git a/tools/libs/vchan/init.c b/tools/libs/vchan/init.c >>> index c8510e6ce9..c6b8674ef5 100644 >>> --- a/tools/libs/vchan/init.c >>> +++ b/tools/libs/vchan/init.c >>> @@ -46,6 +46,8 @@ >>> #include <xen/sys/gntdev.h> >>> #include <libxenvchan.h> >>> >>> +#include "vchan.h" >>> + >>> #ifndef PAGE_SHIFT >>> #define PAGE_SHIFT 12 >>> #endif >>> @@ -251,6 +253,10 @@ static int init_xs_srv(struct libxenvchan *ctrl, int domain, const char* xs_base >>> char ref[16]; >>> char* domid_str = NULL; >>> xs_transaction_t xs_trans = XBT_NULL; >>> + >>> + // store the base path so we can clean up on server closure >> >> Please don't introduce new usages of the C++ comment style. > Most comments in this file are C++ style since libvchan introduced I know. Nevertheless introducing new coding style violations should be avoided, especially as io.c is using /* ... */ already. Juergen
diff --git a/tools/include/libxenvchan.h b/tools/include/libxenvchan.h index d6010b145d..30cc73cf97 100644 --- a/tools/include/libxenvchan.h +++ b/tools/include/libxenvchan.h @@ -86,6 +86,11 @@ struct libxenvchan { int blocking:1; /* communication rings */ struct libxenvchan_ring read, write; + /** + * Base xenstore path for storing ring/event data used by the server + * during cleanup. + * */ + char *xs_path; }; /** diff --git a/tools/libs/vchan/init.c b/tools/libs/vchan/init.c index c8510e6ce9..c6b8674ef5 100644 --- a/tools/libs/vchan/init.c +++ b/tools/libs/vchan/init.c @@ -46,6 +46,8 @@ #include <xen/sys/gntdev.h> #include <libxenvchan.h> +#include "vchan.h" + #ifndef PAGE_SHIFT #define PAGE_SHIFT 12 #endif @@ -251,6 +253,10 @@ static int init_xs_srv(struct libxenvchan *ctrl, int domain, const char* xs_base char ref[16]; char* domid_str = NULL; xs_transaction_t xs_trans = XBT_NULL; + + // store the base path so we can clean up on server closure + ctrl->xs_path = strdup(xs_base); + xs = xs_open(0); if (!xs) goto fail; @@ -298,6 +304,23 @@ retry_transaction: return ret; } +void close_xs_srv(struct libxenvchan *ctrl) +{ + struct xs_handle *xs; + + if (!ctrl->xs_path) + return; + + xs = xs_open(0); + if (!xs) + goto fail; + + xs_rm(xs, XBT_NULL, ctrl->xs_path); + +fail: + free(ctrl->xs_path); +} + static int min_order(size_t siz) { int rv = PAGE_SHIFT; diff --git a/tools/libs/vchan/io.c b/tools/libs/vchan/io.c index da303fbc01..1f201ad554 100644 --- a/tools/libs/vchan/io.c +++ b/tools/libs/vchan/io.c @@ -40,6 +40,8 @@ #include <xenctrl.h> #include <libxenvchan.h> +#include "vchan.h" + #ifndef PAGE_SHIFT #define PAGE_SHIFT 12 #endif @@ -384,5 +386,7 @@ void libxenvchan_close(struct libxenvchan *ctrl) if (ctrl->gnttab) xengnttab_close(ctrl->gnttab); } + if (ctrl->is_server) + close_xs_srv(ctrl); free(ctrl); } diff --git a/tools/libs/vchan/vchan.h b/tools/libs/vchan/vchan.h new file mode 100644 index 0000000000..621016ef42 --- /dev/null +++ b/tools/libs/vchan/vchan.h @@ -0,0 +1,31 @@ +/** + * @file + * @section AUTHORS + * + * Copyright (C) 2021 EPAM Systems Inc. + * + * @section LICENSE + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library 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 + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see <http://www.gnu.org/licenses/>. + * + * @section DESCRIPTION + * + * This file contains common libxenvchan declarations. + */ +#ifndef LIBVCHAN_H +#define LIBVCHAN_H + +void close_xs_srv(struct libxenvchan *ctrl); + +#endif /* LIBVCHAN_H */