From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with UTF8SMTPS id D458260FF1 for ; Thu, 19 Nov 2020 11:59:02 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with UTF8SMTP id C36BA23F4D for ; Thu, 19 Nov 2020 11:58:32 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with UTF8SMTPS id 2D35D23F40 for ; Thu, 19 Nov 2020 11:58:32 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with UTF8SMTP id E34E24369F for ; Thu, 19 Nov 2020 11:58:31 +0100 (CET) To: pve-devel@lists.proxmox.com References: <20201117113836.16895-1-f.ebner@proxmox.com> From: Dominik Csapak Message-ID: <6103b0f7-df72-d5dd-02b8-393a0c1e32e0@proxmox.com> Date: Thu, 19 Nov 2020 11:58:30 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:83.0) Gecko/20100101 Thunderbird/83.0 MIME-Version: 1.0 In-Reply-To: <20201117113836.16895-1-f.ebner@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.339 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [RFC/PATCH qemu] fix #3084: fall back to open-iscsi initiatorname X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 Nov 2020 10:59:02 -0000 idea sounds good, patch looks sane (compiled it, but did not test it though, since i have no iscsi around atm) the only thing that 'could' go wrong afaics, is that we maybe have some codepaths were we only use the default initiatorname and some setups could not do that anymore but this would mean they could not start a vm in the first place... so not an issue Reviewed-By: Dominik Csapak On 11/17/20 12:38 PM, Fabian Ebner wrote: > Fixes vma restore when the target is an iSCSI storage which expects that > initiatorname. Also avoids the need to always explicitly set the initiatorname > in PVE code, thus fixing moving efidisks from and to such iSCSI storages. > > Signed-off-by: Fabian Ebner > --- > > The obvious alternative is to add the option in vma itself. But I'd like to > hear some opinions about this approach first. One downside is that the old > default behavior (i.e. using iqn.2008-11.org.linux-kvmXXX) is overwritten as > long as an initiatorname can be read from /etc/iscsi/initiatorname.iscsi, but > as PVE only reads the initiatorname from there, relying on that behavior seems > to be incompatible with our tooling anyways. > > ...all-back-to-open-iscsi-initiatorname.patch | 71 +++++++++++++++++++ > debian/patches/series | 1 + > 2 files changed, 72 insertions(+) > create mode 100644 debian/patches/pve/0057-PVE-fall-back-to-open-iscsi-initiatorname.patch > > diff --git a/debian/patches/pve/0057-PVE-fall-back-to-open-iscsi-initiatorname.patch b/debian/patches/pve/0057-PVE-fall-back-to-open-iscsi-initiatorname.patch > new file mode 100644 > index 0000000..9f0e822 > --- /dev/null > +++ b/debian/patches/pve/0057-PVE-fall-back-to-open-iscsi-initiatorname.patch > @@ -0,0 +1,71 @@ > +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 > +From: Fabian Ebner > +Date: Tue, 17 Nov 2020 10:51:05 +0100 > +Subject: [PATCH] PVE: fall back to open-iscsi initiatorname > + > +When no explicit option is given, try reading the initiator name from > +/etc/iscsi/initiatorname.iscsi and only use the generic fallback, i.e. > +iqn.2008-11.org.linux-kvmXXX, as a third alternative. > + > +This avoids the need to add an explicit option for vma and to explicitly set it > +for each call to qemu that deals with iSCSI disks, while still allowing to set > +the option if a different name is needed. > + > +According to RFC 3720, an initiator name is at most 223 bytes long, so the > +4 KiB buffer is big enough, even if many whitespaces are used. > + > +Signed-off-by: Fabian Ebner > +--- > + block/iscsi.c | 30 ++++++++++++++++++++++++++++++ > + 1 file changed, 30 insertions(+) > + > +diff --git a/block/iscsi.c b/block/iscsi.c > +index bd2122a3a4..56437d8b99 100644 > +--- a/block/iscsi.c > ++++ b/block/iscsi.c > +@@ -1374,12 +1374,42 @@ static char *get_initiator_name(QemuOpts *opts) > + const char *name; > + char *iscsi_name; > + UuidInfo *uuid_info; > ++ FILE *name_fh; > + > + name = qemu_opt_get(opts, "initiator-name"); > + if (name) { > + return g_strdup(name); > + } > + > ++ name_fh = fopen("/etc/iscsi/initiatorname.iscsi", "r"); > ++ if (name_fh) { > ++ const char *key = "InitiatorName"; > ++ char buffer[4096]; > ++ char *line; > ++ > ++ while ((line = fgets(buffer, sizeof(buffer), name_fh))) { > ++ line = g_strstrip(line); > ++ if (!strncmp(line, key, strlen(key))) { > ++ line = strchr(line, '='); > ++ if (!line || strlen(line) == 1) { > ++ continue; > ++ } > ++ line++; > ++ g_strstrip(line); > ++ if (!strlen(line)) { > ++ continue; > ++ } > ++ name = line; > ++ break; > ++ } > ++ } > ++ fclose(name_fh); > ++ > ++ if (name) { > ++ return g_strdup(name); > ++ } > ++ } > ++ > + uuid_info = qmp_query_uuid(NULL); > + if (strcmp(uuid_info->UUID, UUID_NONE) == 0) { > + name = qemu_get_vm_name(); > +-- > +2.20.1 > + > diff --git a/debian/patches/series b/debian/patches/series > index 0b4ba60..604352b 100644 > --- a/debian/patches/series > +++ b/debian/patches/series > @@ -57,3 +57,4 @@ pve/0053-PVE-fix-and-clean-up-error-handling-for-create_backu.patch > pve/0054-migration-block-dirty-bitmap-fix-larger-granularity-.patch > pve/0055-PVE-Migrate-dirty-bitmap-state-via-savevm.patch > pve/0056-migration-block-dirty-bitmap-migrate-other-bitmaps-e.patch > +pve/0057-PVE-fall-back-to-open-iscsi-initiatorname.patch >