[{"id":1770998,"web_url":"http://patchwork.ozlabs.org/comment/1770998/","msgid":"<20170919125300.GL9536@redhat.com>","list_archive_url":null,"date":"2017-09-19T12:53:00","subject":"Re: [Qemu-devel] [PATCH 4/4] scsi: add persistent reservation\n\tmanager using qemu-pr-helper","submitter":{"id":2694,"url":"http://patchwork.ozlabs.org/api/people/2694/","name":"Daniel P. Berrangé","email":"berrange@redhat.com"},"content":"On Tue, Sep 19, 2017 at 12:24:34PM +0200, Paolo Bonzini wrote:\n> This adds a concrete subclass of pr-manager that talks to qemu-pr-helper.\n> \n> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>\n> ---\n> v1->v2: fixed string property double-free\n>         fixed/cleaned up error handling\n>         handle buffer underrun\n> \n>  scsi/Makefile.objs       |   2 +-\n>  scsi/pr-manager-helper.c | 302 +++++++++++++++++++++++++++++++++++++++++++++++\n>  2 files changed, 303 insertions(+), 1 deletion(-)\n>  create mode 100644 scsi/pr-manager-helper.c\n\n\n> +static int pr_manager_helper_run(PRManager *p,\n> +                                 int fd, struct sg_io_hdr *io_hdr)\n> +{\n> +    PRManagerHelper *pr_mgr = PR_MANAGER_HELPER(p);\n> +\n> +    uint32_t len;\n> +    PRHelperResponse resp;\n> +    int ret;\n> +    int expected_dir;\n> +    int attempts;\n> +    uint8_t cdb[PR_HELPER_CDB_SIZE] = { 0 };\n> +\n> +    if (!io_hdr->cmd_len || io_hdr->cmd_len > PR_HELPER_CDB_SIZE) {\n> +        return -EINVAL;\n> +    }\n> +\n> +    memcpy(cdb, io_hdr->cmdp, io_hdr->cmd_len);\n> +    assert(cdb[0] == PERSISTENT_RESERVE_OUT || cdb[0] == PERSISTENT_RESERVE_IN);\n> +    expected_dir =\n> +        (cdb[0] == PERSISTENT_RESERVE_OUT ? SG_DXFER_TO_DEV : SG_DXFER_FROM_DEV);\n> +    if (io_hdr->dxfer_direction != expected_dir) {\n> +        return -EINVAL;\n> +    }\n> +\n> +    len = scsi_cdb_xfer(cdb);\n> +    if (io_hdr->dxfer_len < len || len > PR_HELPER_DATA_SIZE) {\n> +        return -EINVAL;\n> +    }\n> +\n> +    qemu_mutex_lock(&pr_mgr->lock);\n> +\n> +    /* Try to reconnect while sending the CDB.  */\n> +    for (attempts = 0; attempts < PR_MAX_RECONNECT_ATTEMPTS; attempts++) {\n\nI'm curious why you need to loop here. The helper daemon should be running\nalready, as you're not spawning it on demand IIUC. So it shoudl either\nsucceed first time, or fail every time.\n\n> +        if (!pr_mgr->ioc) {\n> +            ret = pr_manager_helper_initialize(pr_mgr, NULL);\n> +            if (ret < 0) {\n> +                qemu_mutex_unlock(&pr_mgr->lock);\n> +                g_usleep(G_USEC_PER_SEC);\n> +                qemu_mutex_lock(&pr_mgr->lock);\n> +                continue;\n> +            }\n> +        }\n> +\n> +        ret = pr_manager_helper_write(pr_mgr, fd, cdb, ARRAY_SIZE(cdb), NULL);\n> +        if (ret >= 0) {\n> +            break;\n> +        }\n> +    }\n> +    if (ret < 0) {\n> +        goto out;\n> +    }\n\nRegards,\nDaniel","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=berrange@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xxNrt1w0zz9sBZ\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 19 Sep 2017 23:24:43 +1000 (AEST)","from localhost ([::1]:42854 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1duIW0-0007cF-V4\n\tfor incoming@patchwork.ozlabs.org; Tue, 19 Sep 2017 09:24:41 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:47810)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <berrange@redhat.com>) id 1duI1a-0006gt-Qe\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 08:53:16 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <berrange@redhat.com>) id 1duI1Z-0004xj-Ng\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 08:53:14 -0400","from mx1.redhat.com ([209.132.183.28]:54572)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <berrange@redhat.com>)\n\tid 1duI1T-0004td-Fa; Tue, 19 Sep 2017 08:53:07 -0400","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 82165A0379;\n\tTue, 19 Sep 2017 12:53:06 +0000 (UTC)","from redhat.com (unknown [10.42.22.189])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 7DCA160637;\n\tTue, 19 Sep 2017 12:53:03 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 82165A0379","Date":"Tue, 19 Sep 2017 13:53:00 +0100","From":"\"Daniel P. Berrange\" <berrange@redhat.com>","To":"Paolo Bonzini <pbonzini@redhat.com>","Message-ID":"<20170919125300.GL9536@redhat.com>","References":"<20170919102434.21147-1-pbonzini@redhat.com>\n\t<20170919102434.21147-5-pbonzini@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20170919102434.21147-5-pbonzini@redhat.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.39]);\n\tTue, 19 Sep 2017 12:53:06 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 4/4] scsi: add persistent reservation\n\tmanager using qemu-pr-helper","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Reply-To":"\"Daniel P. Berrange\" <berrange@redhat.com>","Cc":"qemu-devel@nongnu.org, qemu-block@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1771001,"web_url":"http://patchwork.ozlabs.org/comment/1771001/","msgid":"<20170919132627.GN9536@redhat.com>","list_archive_url":null,"date":"2017-09-19T13:26:27","subject":"Re: [Qemu-devel] [PATCH 4/4] scsi: add persistent reservation\n\tmanager using qemu-pr-helper","submitter":{"id":2694,"url":"http://patchwork.ozlabs.org/api/people/2694/","name":"Daniel P. Berrangé","email":"berrange@redhat.com"},"content":"On Tue, Sep 19, 2017 at 03:23:09PM +0200, Paolo Bonzini wrote:\n> On 19/09/2017 15:12, Daniel P. Berrange wrote:\n> > On Tue, Sep 19, 2017 at 02:57:00PM +0200, Paolo Bonzini wrote:\n> >> On 19/09/2017 14:53, Daniel P. Berrange wrote:\n> >>>> +    /* Try to reconnect while sending the CDB.  */\n> >>>> +    for (attempts = 0; attempts < PR_MAX_RECONNECT_ATTEMPTS; attempts++) {\n> >>>\n> >>> I'm curious why you need to loop here. The helper daemon should be running\n> >>> already, as you're not spawning it on demand IIUC. So it shoudl either\n> >>> succeed first time, or fail every time.\n> >>\n> >> You're focusing on the usecase where the helper daemon is spawned per-VM\n> >> by the system libvirtd, which I agree is the most important one.\n> >> However, the other usecase is the one with a global daemon, access to\n> >> which is controlled via Unix permissions.  This is not SELinux-friendly,\n> >> but it is nicer for testing and it is also the only possibility for user\n> >> libvirtd.\n> >>\n> >> In that case, upgrading QEMU on the host could result in a \"systemctl\n> >> restart qemu-pr-helper.service\" (or, hopefully unlikely, a crash could\n> >> result in systemd respawning the daemon).  Reconnect is useful in that case.\n> > \n> > If using systemd socket activation and you restart the daemon, the listening\n> > socket should be preserved, so you shouldn't need to reconnect - the client\n> > should get queued until it has started again (likewise on crash).\n> \n> Oh, that's cool.  I didn't know that.  However, systemd socket\n> activation is optional, and it's only a handful of lines so I think it's\n> a bit nicer behavior (chardevs for example have options to reconnect).\n\nThe downside is that if someone forget to start the daemon, or enable\nthe socket,  QEMU will spin for 5 seconds trying to reconnect, instead\nof reporting an error immediately. \n\nRegards,\nDaniel","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx08.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx08.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=berrange@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xxNvy4Q97z9sBZ\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 19 Sep 2017 23:27:26 +1000 (AEST)","from localhost ([::1]:42870 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1duIYe-0001Dj-NL\n\tfor incoming@patchwork.ozlabs.org; Tue, 19 Sep 2017 09:27:24 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:43589)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <berrange@redhat.com>) id 1duIY0-00013f-8g\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 09:26:50 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <berrange@redhat.com>) id 1duIXu-0003bO-Ac\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 09:26:44 -0400","from mx1.redhat.com ([209.132.183.28]:37112)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <berrange@redhat.com>)\n\tid 1duIXo-0003Wf-L1; Tue, 19 Sep 2017 09:26:32 -0400","from smtp.corp.redhat.com\n\t(int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id A7575C050CFC;\n\tTue, 19 Sep 2017 13:26:31 +0000 (UTC)","from redhat.com (unknown [10.42.22.189])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 8651660475;\n\tTue, 19 Sep 2017 13:26:30 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com A7575C050CFC","Date":"Tue, 19 Sep 2017 14:26:27 +0100","From":"\"Daniel P. Berrange\" <berrange@redhat.com>","To":"Paolo Bonzini <pbonzini@redhat.com>","Message-ID":"<20170919132627.GN9536@redhat.com>","References":"<20170919102434.21147-1-pbonzini@redhat.com>\n\t<20170919102434.21147-5-pbonzini@redhat.com>\n\t<20170919125300.GL9536@redhat.com>\n\t<d80b9409-9524-4a05-d0a3-d3e8f23c416b@redhat.com>\n\t<20170919131208.GM9536@redhat.com>\n\t<ddc23258-0e68-f2e3-72a3-c16e443a7143@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<ddc23258-0e68-f2e3-72a3-c16e443a7143@redhat.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.11","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.32]);\n\tTue, 19 Sep 2017 13:26:31 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 4/4] scsi: add persistent reservation\n\tmanager using qemu-pr-helper","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Reply-To":"\"Daniel P. Berrange\" <berrange@redhat.com>","Cc":"qemu-devel@nongnu.org, qemu-block@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1771003,"web_url":"http://patchwork.ozlabs.org/comment/1771003/","msgid":"<d80b9409-9524-4a05-d0a3-d3e8f23c416b@redhat.com>","list_archive_url":null,"date":"2017-09-19T12:57:00","subject":"Re: [Qemu-devel] [PATCH 4/4] scsi: add persistent reservation\n\tmanager using qemu-pr-helper","submitter":{"id":2701,"url":"http://patchwork.ozlabs.org/api/people/2701/","name":"Paolo Bonzini","email":"pbonzini@redhat.com"},"content":"On 19/09/2017 14:53, Daniel P. Berrange wrote:\n>> +    /* Try to reconnect while sending the CDB.  */\n>> +    for (attempts = 0; attempts < PR_MAX_RECONNECT_ATTEMPTS; attempts++) {\n>\n> I'm curious why you need to loop here. The helper daemon should be running\n> already, as you're not spawning it on demand IIUC. So it shoudl either\n> succeed first time, or fail every time.\n\nYou're focusing on the usecase where the helper daemon is spawned per-VM\nby the system libvirtd, which I agree is the most important one.\nHowever, the other usecase is the one with a global daemon, access to\nwhich is controlled via Unix permissions.  This is not SELinux-friendly,\nbut it is nicer for testing and it is also the only possibility for user\nlibvirtd.\n\nIn that case, upgrading QEMU on the host could result in a \"systemctl\nrestart qemu-pr-helper.service\" (or, hopefully unlikely, a crash could\nresult in systemd respawning the daemon).  Reconnect is useful in that case.\n\nPaolo","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=pbonzini@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xxNx46z2Fz9sBZ\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 19 Sep 2017 23:28:24 +1000 (AEST)","from localhost ([::1]:42872 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1duIZa-0001vI-PY\n\tfor incoming@patchwork.ozlabs.org; Tue, 19 Sep 2017 09:28:22 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:50787)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <pbonzini@redhat.com>) id 1duI5Y-0001nS-61\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 08:57:21 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <pbonzini@redhat.com>) id 1duI5T-0007vy-8q\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 08:57:20 -0400","from mx1.redhat.com ([209.132.183.28]:49708)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <pbonzini@redhat.com>)\n\tid 1duI5I-0007qL-Bu; Tue, 19 Sep 2017 08:57:04 -0400","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 495E1806C2;\n\tTue, 19 Sep 2017 12:57:03 +0000 (UTC)","from [10.36.117.61] (ovpn-117-61.ams2.redhat.com [10.36.117.61])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 7A8FB5D6A4;\n\tTue, 19 Sep 2017 12:57:02 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 495E1806C2","To":"\"Daniel P. Berrange\" <berrange@redhat.com>","References":"<20170919102434.21147-1-pbonzini@redhat.com>\n\t<20170919102434.21147-5-pbonzini@redhat.com>\n\t<20170919125300.GL9536@redhat.com>","From":"Paolo Bonzini <pbonzini@redhat.com>","Message-ID":"<d80b9409-9524-4a05-d0a3-d3e8f23c416b@redhat.com>","Date":"Tue, 19 Sep 2017 14:57:00 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170919125300.GL9536@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.26]);\n\tTue, 19 Sep 2017 12:57:03 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 4/4] scsi: add persistent reservation\n\tmanager using qemu-pr-helper","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"qemu-devel@nongnu.org, qemu-block@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1771011,"web_url":"http://patchwork.ozlabs.org/comment/1771011/","msgid":"<20170919131208.GM9536@redhat.com>","list_archive_url":null,"date":"2017-09-19T13:12:08","subject":"Re: [Qemu-devel] [PATCH 4/4] scsi: add persistent reservation\n\tmanager using qemu-pr-helper","submitter":{"id":2694,"url":"http://patchwork.ozlabs.org/api/people/2694/","name":"Daniel P. Berrangé","email":"berrange@redhat.com"},"content":"On Tue, Sep 19, 2017 at 02:57:00PM +0200, Paolo Bonzini wrote:\n> On 19/09/2017 14:53, Daniel P. Berrange wrote:\n> >> +    /* Try to reconnect while sending the CDB.  */\n> >> +    for (attempts = 0; attempts < PR_MAX_RECONNECT_ATTEMPTS; attempts++) {\n> >\n> > I'm curious why you need to loop here. The helper daemon should be running\n> > already, as you're not spawning it on demand IIUC. So it shoudl either\n> > succeed first time, or fail every time.\n> \n> You're focusing on the usecase where the helper daemon is spawned per-VM\n> by the system libvirtd, which I agree is the most important one.\n> However, the other usecase is the one with a global daemon, access to\n> which is controlled via Unix permissions.  This is not SELinux-friendly,\n> but it is nicer for testing and it is also the only possibility for user\n> libvirtd.\n> \n> In that case, upgrading QEMU on the host could result in a \"systemctl\n> restart qemu-pr-helper.service\" (or, hopefully unlikely, a crash could\n> result in systemd respawning the daemon).  Reconnect is useful in that case.\n\nIf using systemd socket activation and you restart the daemon, the listening\nsocket should be preserved, so you shouldn't need to reconnect - the client\nshould get queued until it has started again (likewise on crash).\n\nRegards,\nDaniel","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx03.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx03.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=berrange@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xxP2H037Hz9sBZ\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 19 Sep 2017 23:32:55 +1000 (AEST)","from localhost ([::1]:42903 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1duIdx-0005h4-1a\n\tfor incoming@patchwork.ozlabs.org; Tue, 19 Sep 2017 09:32:53 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:33305)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <berrange@redhat.com>) id 1duIKC-0006QJ-GA\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 09:12:32 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <berrange@redhat.com>) id 1duIK8-000249-Ge\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 09:12:28 -0400","from mx1.redhat.com ([209.132.183.28]:51658)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <berrange@redhat.com>)\n\tid 1duIJx-0001ws-Eg; Tue, 19 Sep 2017 09:12:13 -0400","from smtp.corp.redhat.com\n\t(int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 8D4D17E44D;\n\tTue, 19 Sep 2017 13:12:12 +0000 (UTC)","from redhat.com (unknown [10.42.22.189])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 9DEE85D986;\n\tTue, 19 Sep 2017 13:12:11 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 8D4D17E44D","Date":"Tue, 19 Sep 2017 14:12:08 +0100","From":"\"Daniel P. Berrange\" <berrange@redhat.com>","To":"Paolo Bonzini <pbonzini@redhat.com>","Message-ID":"<20170919131208.GM9536@redhat.com>","References":"<20170919102434.21147-1-pbonzini@redhat.com>\n\t<20170919102434.21147-5-pbonzini@redhat.com>\n\t<20170919125300.GL9536@redhat.com>\n\t<d80b9409-9524-4a05-d0a3-d3e8f23c416b@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<d80b9409-9524-4a05-d0a3-d3e8f23c416b@redhat.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.14","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.27]);\n\tTue, 19 Sep 2017 13:12:12 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 4/4] scsi: add persistent reservation\n\tmanager using qemu-pr-helper","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Reply-To":"\"Daniel P. Berrange\" <berrange@redhat.com>","Cc":"qemu-devel@nongnu.org, qemu-block@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1771016,"web_url":"http://patchwork.ozlabs.org/comment/1771016/","msgid":"<20170919125051.GK9536@redhat.com>","list_archive_url":null,"date":"2017-09-19T12:50:51","subject":"Re: [Qemu-devel] [PATCH 4/4] scsi: add persistent reservation\n\tmanager using qemu-pr-helper","submitter":{"id":2694,"url":"http://patchwork.ozlabs.org/api/people/2694/","name":"Daniel P. Berrangé","email":"berrange@redhat.com"},"content":"On Tue, Sep 19, 2017 at 12:24:34PM +0200, Paolo Bonzini wrote:\n> This adds a concrete subclass of pr-manager that talks to qemu-pr-helper.\n> \n> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>\n> ---\n> v1->v2: fixed string property double-free\n>         fixed/cleaned up error handling\n>         handle buffer underrun\n> \n>  scsi/Makefile.objs       |   2 +-\n>  scsi/pr-manager-helper.c | 302 +++++++++++++++++++++++++++++++++++++++++++++++\n>  2 files changed, 303 insertions(+), 1 deletion(-)\n>  create mode 100644 scsi/pr-manager-helper.c\n> \n> diff --git a/scsi/Makefile.objs b/scsi/Makefile.objs\n> index 5496d2ae6a..4d25e476cf 100644\n> --- a/scsi/Makefile.objs\n> +++ b/scsi/Makefile.objs\n> @@ -1,3 +1,3 @@\n>  block-obj-y += utils.o\n>  \n> -block-obj-$(CONFIG_LINUX) += pr-manager.o\n> +block-obj-$(CONFIG_LINUX) += pr-manager.o pr-manager-helper.o\n> diff --git a/scsi/pr-manager-helper.c b/scsi/pr-manager-helper.c\n> new file mode 100644\n> index 0000000000..063fd35dee\n> --- /dev/null\n> +++ b/scsi/pr-manager-helper.c\n\n> +/* Called with lock held.  */\n> +static int pr_manager_helper_read(PRManagerHelper *pr_mgr,\n> +                                  void *buf, int sz, Error **errp)\n> +{\n> +    ssize_t r = qio_channel_read_all(pr_mgr->ioc, buf, sz, errp);\n\nThe return value isn't ssize_t - it just returns 0 on success, -1 on\nerror, never a bytes count since it always reads all requested bytes\nand treats EOF as an error condition.\n\n> +\n> +    if (r < 0) {\n> +        object_unref(OBJECT(pr_mgr->ioc));\n> +        pr_mgr->ioc = NULL;\n> +        return -EINVAL;\n> +    }\n> +\n> +    return 0;\n> +}\n> +\n\nRegards,\nDaniel","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx05.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx05.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=berrange@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xxP4r6pWGz9sMN\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 19 Sep 2017 23:35:08 +1000 (AEST)","from localhost ([::1]:42912 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1duIg7-0007Fo-2C\n\tfor incoming@patchwork.ozlabs.org; Tue, 19 Sep 2017 09:35:07 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:46281)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <berrange@redhat.com>) id 1duHzX-0004tV-HX\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 08:51:14 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <berrange@redhat.com>) id 1duHzR-0003f1-MW\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 08:51:07 -0400","from mx1.redhat.com ([209.132.183.28]:49436)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <berrange@redhat.com>)\n\tid 1duHzM-0003aX-1y; Tue, 19 Sep 2017 08:50:56 -0400","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id DBD8815561;\n\tTue, 19 Sep 2017 12:50:54 +0000 (UTC)","from redhat.com (unknown [10.42.22.189])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id C62755D6A8;\n\tTue, 19 Sep 2017 12:50:53 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com DBD8815561","Date":"Tue, 19 Sep 2017 13:50:51 +0100","From":"\"Daniel P. Berrange\" <berrange@redhat.com>","To":"Paolo Bonzini <pbonzini@redhat.com>","Message-ID":"<20170919125051.GK9536@redhat.com>","References":"<20170919102434.21147-1-pbonzini@redhat.com>\n\t<20170919102434.21147-5-pbonzini@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20170919102434.21147-5-pbonzini@redhat.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.29]);\n\tTue, 19 Sep 2017 12:50:55 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 4/4] scsi: add persistent reservation\n\tmanager using qemu-pr-helper","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Reply-To":"\"Daniel P. Berrange\" <berrange@redhat.com>","Cc":"qemu-devel@nongnu.org, qemu-block@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1771025,"web_url":"http://patchwork.ozlabs.org/comment/1771025/","msgid":"<ddc23258-0e68-f2e3-72a3-c16e443a7143@redhat.com>","list_archive_url":null,"date":"2017-09-19T13:23:09","subject":"Re: [Qemu-devel] [PATCH 4/4] scsi: add persistent reservation\n\tmanager using qemu-pr-helper","submitter":{"id":2701,"url":"http://patchwork.ozlabs.org/api/people/2701/","name":"Paolo Bonzini","email":"pbonzini@redhat.com"},"content":"On 19/09/2017 15:12, Daniel P. Berrange wrote:\n> On Tue, Sep 19, 2017 at 02:57:00PM +0200, Paolo Bonzini wrote:\n>> On 19/09/2017 14:53, Daniel P. Berrange wrote:\n>>>> +    /* Try to reconnect while sending the CDB.  */\n>>>> +    for (attempts = 0; attempts < PR_MAX_RECONNECT_ATTEMPTS; attempts++) {\n>>>\n>>> I'm curious why you need to loop here. The helper daemon should be running\n>>> already, as you're not spawning it on demand IIUC. So it shoudl either\n>>> succeed first time, or fail every time.\n>>\n>> You're focusing on the usecase where the helper daemon is spawned per-VM\n>> by the system libvirtd, which I agree is the most important one.\n>> However, the other usecase is the one with a global daemon, access to\n>> which is controlled via Unix permissions.  This is not SELinux-friendly,\n>> but it is nicer for testing and it is also the only possibility for user\n>> libvirtd.\n>>\n>> In that case, upgrading QEMU on the host could result in a \"systemctl\n>> restart qemu-pr-helper.service\" (or, hopefully unlikely, a crash could\n>> result in systemd respawning the daemon).  Reconnect is useful in that case.\n> \n> If using systemd socket activation and you restart the daemon, the listening\n> socket should be preserved, so you shouldn't need to reconnect - the client\n> should get queued until it has started again (likewise on crash).\n\nOh, that's cool.  I didn't know that.  However, systemd socket\nactivation is optional, and it's only a handful of lines so I think it's\na bit nicer behavior (chardevs for example have options to reconnect).\n\nPaolo","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=pbonzini@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xxPBR5P3Qz9rxl\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 19 Sep 2017 23:39:59 +1000 (AEST)","from localhost ([::1]:42946 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1duIkn-0002Rg-Q5\n\tfor incoming@patchwork.ozlabs.org; Tue, 19 Sep 2017 09:39:57 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:41862)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <pbonzini@redhat.com>) id 1duIUq-00077v-7T\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 09:23:29 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <pbonzini@redhat.com>) id 1duIUk-0001bb-Jl\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 09:23:28 -0400","from mx1.redhat.com ([209.132.183.28]:1822)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <pbonzini@redhat.com>)\n\tid 1duIUb-0001V1-Cn; Tue, 19 Sep 2017 09:23:13 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 7837B25BA6;\n\tTue, 19 Sep 2017 13:23:12 +0000 (UTC)","from [10.36.117.61] (ovpn-117-61.ams2.redhat.com [10.36.117.61])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 84D8C65E84;\n\tTue, 19 Sep 2017 13:23:10 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 7837B25BA6","To":"\"Daniel P. Berrange\" <berrange@redhat.com>","References":"<20170919102434.21147-1-pbonzini@redhat.com>\n\t<20170919102434.21147-5-pbonzini@redhat.com>\n\t<20170919125300.GL9536@redhat.com>\n\t<d80b9409-9524-4a05-d0a3-d3e8f23c416b@redhat.com>\n\t<20170919131208.GM9536@redhat.com>","From":"Paolo Bonzini <pbonzini@redhat.com>","Message-ID":"<ddc23258-0e68-f2e3-72a3-c16e443a7143@redhat.com>","Date":"Tue, 19 Sep 2017 15:23:09 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170919131208.GM9536@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.39]);\n\tTue, 19 Sep 2017 13:23:12 +0000 (UTC)","Content-Transfer-Encoding":"quoted-printable","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 4/4] scsi: add persistent reservation\n\tmanager using qemu-pr-helper","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"qemu-devel@nongnu.org, qemu-block@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1771085,"web_url":"http://patchwork.ozlabs.org/comment/1771085/","msgid":"<c01b9beb-7f85-32ed-2958-f1a688ba3478@redhat.com>","list_archive_url":null,"date":"2017-09-19T14:32:05","subject":"Re: [Qemu-devel] [PATCH 4/4] scsi: add persistent reservation\n\tmanager using qemu-pr-helper","submitter":{"id":2701,"url":"http://patchwork.ozlabs.org/api/people/2701/","name":"Paolo Bonzini","email":"pbonzini@redhat.com"},"content":"On 19/09/2017 15:26, Daniel P. Berrange wrote:\n> On Tue, Sep 19, 2017 at 03:23:09PM +0200, Paolo Bonzini wrote:\n>> On 19/09/2017 15:12, Daniel P. Berrange wrote:\n>>> On Tue, Sep 19, 2017 at 02:57:00PM +0200, Paolo Bonzini wrote:\n>>>> On 19/09/2017 14:53, Daniel P. Berrange wrote:\n>>>>>> +    /* Try to reconnect while sending the CDB.  */\n>>>>>> +    for (attempts = 0; attempts < PR_MAX_RECONNECT_ATTEMPTS; attempts++) {\n>>>>>\n>>>>> I'm curious why you need to loop here. The helper daemon should be running\n>>>>> already, as you're not spawning it on demand IIUC. So it shoudl either\n>>>>> succeed first time, or fail every time.\n>>>>\n>>>> You're focusing on the usecase where the helper daemon is spawned per-VM\n>>>> by the system libvirtd, which I agree is the most important one.\n>>>> However, the other usecase is the one with a global daemon, access to\n>>>> which is controlled via Unix permissions.  This is not SELinux-friendly,\n>>>> but it is nicer for testing and it is also the only possibility for user\n>>>> libvirtd.\n>>>>\n>>>> In that case, upgrading QEMU on the host could result in a \"systemctl\n>>>> restart qemu-pr-helper.service\" (or, hopefully unlikely, a crash could\n>>>> result in systemd respawning the daemon).  Reconnect is useful in that case.\n>>>\n>>> If using systemd socket activation and you restart the daemon, the listening\n>>> socket should be preserved, so you shouldn't need to reconnect - the client\n>>> should get queued until it has started again (likewise on crash).\n>>\n>> Oh, that's cool.  I didn't know that.  However, systemd socket\n>> activation is optional, and it's only a handful of lines so I think it's\n>> a bit nicer behavior (chardevs for example have options to reconnect).\n> \n> The downside is that if someone forget to start the daemon, or enable\n> the socket,  QEMU will spin for 5 seconds trying to reconnect, instead\n> of reporting an error immediately. \n\nNot exactly:\n\n- the daemon must be present at startup.  Reconnect is only tried when\nsending a command.\n\n- it's not a busy wait, pr_manager_helper_run runs in an\nutil/thread-pool.c worker thread (see pr_manager_execute in patch 1).\n\nPaolo","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=pbonzini@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xxQN16jZcz9s7h\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed, 20 Sep 2017 00:33:21 +1000 (AEST)","from localhost ([::1]:43254 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1duJaS-0002S7-28\n\tfor incoming@patchwork.ozlabs.org; Tue, 19 Sep 2017 10:33:20 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:56695)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <pbonzini@redhat.com>) id 1duJZb-0002IZ-9O\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 10:32:33 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <pbonzini@redhat.com>) id 1duJZV-0007wV-MD\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 10:32:27 -0400","from mx1.redhat.com ([209.132.183.28]:25005)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <pbonzini@redhat.com>)\n\tid 1duJZI-0007l8-Gp; Tue, 19 Sep 2017 10:32:08 -0400","from smtp.corp.redhat.com\n\t(int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 986A15F7AE;\n\tTue, 19 Sep 2017 14:32:07 +0000 (UTC)","from [10.36.117.61] (ovpn-117-61.ams2.redhat.com [10.36.117.61])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id B6C065D980;\n\tTue, 19 Sep 2017 14:32:06 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 986A15F7AE","To":"\"Daniel P. Berrange\" <berrange@redhat.com>","References":"<20170919102434.21147-1-pbonzini@redhat.com>\n\t<20170919102434.21147-5-pbonzini@redhat.com>\n\t<20170919125300.GL9536@redhat.com>\n\t<d80b9409-9524-4a05-d0a3-d3e8f23c416b@redhat.com>\n\t<20170919131208.GM9536@redhat.com>\n\t<ddc23258-0e68-f2e3-72a3-c16e443a7143@redhat.com>\n\t<20170919132627.GN9536@redhat.com>","From":"Paolo Bonzini <pbonzini@redhat.com>","Message-ID":"<c01b9beb-7f85-32ed-2958-f1a688ba3478@redhat.com>","Date":"Tue, 19 Sep 2017 16:32:05 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170919132627.GN9536@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.14","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.39]);\n\tTue, 19 Sep 2017 14:32:07 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 4/4] scsi: add persistent reservation\n\tmanager using qemu-pr-helper","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"qemu-devel@nongnu.org, qemu-block@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1772963,"web_url":"http://patchwork.ozlabs.org/comment/1772963/","msgid":"<3952279a-b001-db23-ce5f-763bbcadd996@redhat.com>","list_archive_url":null,"date":"2017-09-21T16:20:18","subject":"Re: [Qemu-devel] [Qemu-block] [PATCH 4/4] scsi: add persistent\n\treservation manager using qemu-pr-helper","submitter":{"id":2701,"url":"http://patchwork.ozlabs.org/api/people/2701/","name":"Paolo Bonzini","email":"pbonzini@redhat.com"},"content":"On 19/09/2017 15:23, Paolo Bonzini wrote:\n> On 19/09/2017 15:12, Daniel P. Berrange wrote:\n>> On Tue, Sep 19, 2017 at 02:57:00PM +0200, Paolo Bonzini wrote:\n>>> On 19/09/2017 14:53, Daniel P. Berrange wrote:\n>>>>> +    /* Try to reconnect while sending the CDB.  */\n>>>>> +    for (attempts = 0; attempts < PR_MAX_RECONNECT_ATTEMPTS; attempts++) {\n>>>>\n>>>> I'm curious why you need to loop here. The helper daemon should be running\n>>>> already, as you're not spawning it on demand IIUC. So it shoudl either\n>>>> succeed first time, or fail every time.\n>>>\n>>> You're focusing on the usecase where the helper daemon is spawned per-VM\n>>> by the system libvirtd, which I agree is the most important one.\n>>> However, the other usecase is the one with a global daemon, access to\n>>> which is controlled via Unix permissions.  This is not SELinux-friendly,\n>>> but it is nicer for testing and it is also the only possibility for user\n>>> libvirtd.\n>>>\n>>> In that case, upgrading QEMU on the host could result in a \"systemctl\n>>> restart qemu-pr-helper.service\" (or, hopefully unlikely, a crash could\n>>> result in systemd respawning the daemon).  Reconnect is useful in that case.\n>>\n>> If using systemd socket activation and you restart the daemon, the listening\n>> socket should be preserved, so you shouldn't need to reconnect - the client\n>> should get queued until it has started again (likewise on crash).\n> \n> Oh, that's cool.  I didn't know that.\n\n... wait, I do need to reconnect, because the active socket is closed.\nI only don't need to retry and wait between the retries.\n\nPaolo","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xyhgW44N3z9t42\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri, 22 Sep 2017 02:21:11 +1000 (AEST)","from localhost ([::1]:54555 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dv4Dt-0002nU-4N\n\tfor incoming@patchwork.ozlabs.org; Thu, 21 Sep 2017 12:21:09 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:54431)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <pbonzini@redhat.com>) id 1dv4DE-0002ka-IB\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 12:20:34 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <pbonzini@redhat.com>) id 1dv4DA-00026x-BR\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 12:20:28 -0400","from mail-wr0-f180.google.com ([209.85.128.180]:53737)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)\n\t(Exim 4.71) (envelope-from <pbonzini@redhat.com>) id 1dv4DA-00025H-4f\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 12:20:24 -0400","by mail-wr0-f180.google.com with SMTP id l22so5039150wrc.10\n\tfor <qemu-devel@nongnu.org>; Thu, 21 Sep 2017 09:20:22 -0700 (PDT)","from [192.168.10.165]\n\t(dynamic-adsl-78-12-246-117.clienti.tiscali.it.\n\t[78.12.246.117]) by smtp.gmail.com with ESMTPSA id\n\tc11sm1471226wrb.14.2017.09.21.09.20.19\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tThu, 21 Sep 2017 09:20:19 -0700 (PDT)"],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:subject:from:to:cc:references:message-id:date\n\t:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=Tghk6zBpfpJ82UtjzBNnTsXkyV+UoW/FHOFBGyY8M68=;\n\tb=pC/w7PKbJoEPOAr7qdaz8xThaeFiJbEe9FYlkIsu+LN3igCwjm3dmbTEkQAkzvM8Rx\n\tGQdcE7hiaxopRHYvNO1vHdZ5UbboP4pD2dDtGadqo4NSOhkOoVnS3L+qlHxUjw3yhTXi\n\t4YEMYEo3c+aOPDFEN8dZ+C5iAoQaiv0gpNZxgwZOZBGo4MFCuVW5oLyVQuEjT+Axz0MY\n\tzuJDOcc7A5ZublL7rP2Nqve6n0eKiHC04nQZaC5iKQURXaH4NQGZSsx6MVntJ8GstEqI\n\tJDiSGQRPqENOo9GiAE2NFqsjH5w/mbtRR81AW9wtmB6J8IkAHp1Paw9qiZlcpOQBVi7t\n\taphw==","X-Gm-Message-State":"AHPjjUgdXZbHrKAqKc7JdQ/y4lmGTePo8shkonxM38vyFXUemjth+ehO\n\tbKz1QpTjNpJqyuRNTpeFSTtUbQ==","X-Google-Smtp-Source":"AOwi7QB5fH7M/QcDCf80U0oIKaHogFJi2x05uSrpcY0owUKB75D3+IzI5EYh2qvGTJfzbdA3E78a6w==","X-Received":"by 10.223.196.161 with SMTP id m30mr2338260wrf.187.1506010821538;\n\tThu, 21 Sep 2017 09:20:21 -0700 (PDT)","From":"Paolo Bonzini <pbonzini@redhat.com>","To":"\"Daniel P. Berrange\" <berrange@redhat.com>","References":"<20170919102434.21147-1-pbonzini@redhat.com>\n\t<20170919102434.21147-5-pbonzini@redhat.com>\n\t<20170919125300.GL9536@redhat.com>\n\t<d80b9409-9524-4a05-d0a3-d3e8f23c416b@redhat.com>\n\t<20170919131208.GM9536@redhat.com>\n\t<ddc23258-0e68-f2e3-72a3-c16e443a7143@redhat.com>","Message-ID":"<3952279a-b001-db23-ce5f-763bbcadd996@redhat.com>","Date":"Thu, 21 Sep 2017 18:20:18 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<ddc23258-0e68-f2e3-72a3-c16e443a7143@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.85.128.180","Subject":"Re: [Qemu-devel] [Qemu-block] [PATCH 4/4] scsi: add persistent\n\treservation manager using qemu-pr-helper","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"qemu-devel@nongnu.org, qemu-block@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}}]