[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] tools: remove xenstore entries on vchan server closure


  • To: Jason Andryuk <jandryuk@xxxxxxxxx>
  • From: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>
  • Date: Tue, 15 Feb 2022 14:54:16 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=epam.com; dmarc=pass action=none header.from=epam.com; dkim=pass header.d=epam.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=grJ8uqXQhD6KACc2B+omCmEyqZdwimffM0KIj5QAK9I=; b=bGUJdkei/JsXYvG06l84FauzMpVD+SnWqDYwDUa+XxOV00WwxYO0MJh0WxcLdncsDV7EAKOaLxW476A+WwUDU/Q6sAuCoxkc5EYJb+ksfRPOBGoT8YbWbBgfMWEDTXSl7bs6+RYCkzVyRikQgLBGy9nIxAoGEDPXMVRT95RJrccduep5has/sGk4VmoWK9EdNjTpo78OEg4G2t5KM72SPxNCpcWtqDePWK7QmGq8lRFdhJ6S5JAXN1IoeYGfsCVWhjzlLw39Owz6y5pdcZ3Bsb6WO23J1/m/EIxrolFct8JXlmAlwfQc226c9oVFu+MCVrWMvl3awKXNcRCtJa6IBw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=SZJBzpPAZgDVRdQ+Rhx8khYagEPiY19wzqgl5VOyX53E7Z1X2N56XlFKDKPK9bHaO1P2xhUKsPfK/CEc5rEhHj0XYXZ+JigQ6HvLJ6/uHF+4M4D8u2XcOXGuy2MYSO4DB2fuuz6CW/8lS/9aNj/B7BQcPGSJdg4KDoxoN2d1M0c7SjbxlGHKfSanJgbnd6Ad1Jgb7Q2iHYJ6QmWOqNxssrurHJtQCw0K48DDqgbiE9w1Q/6SaIQlfukvtCqCpbRy/LHck47QBIOd4OPpvpznIoPSyHiyiJeGtl+nZjTXSFAlF3Sy1DGNsmQex5n1xmXapgT/ienUdGQK+UHTgXmNfQ==
  • Cc: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Juergen Gross <jgross@xxxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>
  • Delivery-date: Tue, 15 Feb 2022 14:54:34 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHX7cJnOJurcgYme0yvunOBFlJKtayVGKiAgAADoIA=
  • Thread-topic: [PATCH] tools: remove xenstore entries on vchan server closure


On 15.02.22 16:41, Jason Andryuk wrote:
> On Fri, Dec 10, 2021 at 7:35 AM Oleksandr Andrushchenko
> <andr2000@xxxxxxxxx> wrote:
>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
>>
>> 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@xxxxxxxx>
>> ---
>>   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 d6010b145df2..30cc73cf97e3 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 c8510e6ce98a..c6b8674ef541 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);
> You don't check for NULL here, but you do check for NULL in
> close_xs_srv().  I guess it's okay, since it does the right thing.
> But I think it would be more robust to check for NULL here.  Is there
> a specific reason you wrote it this way?  Otherwise it looks good.
It does need a NULL check, thanks
It is after writing code with all those allocations and garbage collector
in the tools stack when allocations "don't fail" ;)
But this is indeed not the case here and needs a proper check
I'll wait for other comments and send v2
>
> Regards,
> Jason
Thank you,
Oleksandr

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.