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 ESMTPS id 7F55368B19 for ; Wed, 10 Feb 2021 12:05:09 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 669B799E9 for ; Wed, 10 Feb 2021 12:05:09 +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 ESMTPS id 4D84399DF for ; Wed, 10 Feb 2021 12:05:07 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id DDF62461A4 for ; Wed, 10 Feb 2021 12:05:06 +0100 (CET) To: Proxmox VE development discussion , =?UTF-8?Q?Fabian_Gr=c3=bcnbichler?= References: <20210208130835.2512356-1-f.gruenbichler@proxmox.com> <20210208130835.2512356-3-f.gruenbichler@proxmox.com> From: Stefan Reiter Message-ID: <789a02a5-d5c4-b9d1-b8e8-e99569d0639b@proxmox.com> Date: Wed, 10 Feb 2021 12:05:05 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 MIME-Version: 1.0 In-Reply-To: <20210208130835.2512356-3-f.gruenbichler@proxmox.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -1.453 Adjusted score from AWL reputation of From: address KAM_ASCII_DIVIDERS 0.8 Spam that uses ascii formatting tricks KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.265 Looks like a legit reply (A) 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] [PATCH qemu] pbs: add master key support 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: Wed, 10 Feb 2021 11:05:09 -0000 Patch looks good in general, but the added file does not follow our formatting for the other patches. I'd prefer to keep them the same, or at least applicable with 'git am'. Since one of us is going to have to rebase anyway, I can also send along a fixed up version with v2 of my 5.2 series if you want. On 08/02/2021 14:08, Fabian Grünbichler wrote: > this requires a new enough libproxmox-backup-qemu0, and allows querying > from the PVE side to avoid QMP calls with unsupported parameters. > > Signed-off-by: Fabian Grünbichler > --- > > Notes: > requires versioned build and runtime dep on libproxmox-backup-qemu with changed API for masterkey support > > .../pve/0059-pbs-backup-add-masterkey.patch | 96 +++++++++++++++++++ > debian/patches/series | 1 + > 2 files changed, 97 insertions(+) > create mode 100644 debian/patches/pve/0059-pbs-backup-add-masterkey.patch > > diff --git a/debian/patches/pve/0059-pbs-backup-add-masterkey.patch b/debian/patches/pve/0059-pbs-backup-add-masterkey.patch > new file mode 100644 > index 0000000..12fce05 > --- /dev/null > +++ b/debian/patches/pve/0059-pbs-backup-add-masterkey.patch > @@ -0,0 +1,96 @@ > +Index: qemu/pve-backup.c > +=================================================================== > +--- qemu.orig/pve-backup.c > ++++ qemu/pve-backup.c > +@@ -539,6 +539,8 @@ typedef struct QmpBackupTask { > + const char *keyfile; > + bool has_key_password; > + const char *key_password; > ++ bool has_master_keyfile; > ++ const char *master_keyfile; > + bool has_backup_id; > + const char *backup_id; > + bool has_backup_time; > +@@ -710,6 +712,7 @@ static void coroutine_fn pvebackup_co_pr > + task->has_password ? task->password : NULL, > + task->has_keyfile ? task->keyfile : NULL, > + task->has_key_password ? task->key_password : NULL, > ++ task->has_master_keyfile ? task->master_keyfile : NULL, > + task->has_compress ? task->compress : true, > + task->has_encrypt ? task->encrypt : task->has_keyfile, > + task->has_fingerprint ? task->fingerprint : NULL, > +@@ -989,6 +992,7 @@ UuidInfo *qmp_backup( > + bool has_password, const char *password, > + bool has_keyfile, const char *keyfile, > + bool has_key_password, const char *key_password, > ++ bool has_master_keyfile, const char *master_keyfile, > + bool has_fingerprint, const char *fingerprint, > + bool has_backup_id, const char *backup_id, > + bool has_backup_time, int64_t backup_time, > +@@ -1009,6 +1013,8 @@ UuidInfo *qmp_backup( > + .keyfile = keyfile, > + .has_key_password = has_key_password, > + .key_password = key_password, > ++ .has_master_keyfile = has_master_keyfile, > ++ .master_keyfile = master_keyfile, > + .has_fingerprint = has_fingerprint, > + .fingerprint = fingerprint, > + .has_backup_id = has_backup_id, > +@@ -1131,5 +1137,6 @@ ProxmoxSupportStatus *qmp_query_proxmox_ > + ret->pbs_dirty_bitmap = true; > + ret->query_bitmap_info = true; > + ret->pbs_dirty_bitmap_migration = true; > ++ ret->pbs_masterkey = true; > + return ret; > + } > +Index: qemu/block/monitor/block-hmp-cmds.c > +=================================================================== > +--- qemu.orig/block/monitor/block-hmp-cmds.c > ++++ qemu/block/monitor/block-hmp-cmds.c > +@@ -1035,6 +1035,7 @@ void hmp_backup(Monitor *mon, const QDic > + false, NULL, // PBS password > + false, NULL, // PBS keyfile > + false, NULL, // PBS key_password > ++ false, NULL, // PBS master_keyfile > + false, NULL, // PBS fingerprint > + false, NULL, // PBS backup-id > + false, 0, // PBS backup-time > +Index: qemu/qapi/block-core.json > +=================================================================== > +--- qemu.orig/qapi/block-core.json > ++++ qemu/qapi/block-core.json > +@@ -827,6 +827,8 @@ > + # > + # @key-password: password for keyfile (optional for format 'pbs') > + # > ++# @master_keyfile: PEM-formatted master public keyfile (optional for format 'pbs') > ++# please use master-keyfile with a dash > + # @fingerprint: server cert fingerprint (optional for format 'pbs') > + # > + # @backup-id: backup ID (required for format 'pbs') > +@@ -846,6 +848,7 @@ > + '*password': 'str', > + '*keyfile': 'str', > + '*key-password': 'str', > ++ '*master_keyfile': 'str', here too Upstream seems to use _ in some places, but at least keep it consistent in our code ;) > + '*fingerprint': 'str', > + '*backup-id': 'str', > + '*backup-time': 'int', > +@@ -895,6 +898,9 @@ > + # migration cap if this is false/unset may lead > + # to crashes on migration! > + # > ++# @pbs-masterkey: True if the QMP backup call supports the 'master_keyfile' > ++# parameter. > ++# > + # @pbs-library-version: Running version of libproxmox-backup-qemu0 library. > + # > + ## > +@@ -902,6 +908,7 @@ > + 'data': { 'pbs-dirty-bitmap': 'bool', > + 'query-bitmap-info': 'bool', > + 'pbs-dirty-bitmap-migration': 'bool', > ++ 'pbs-masterkey': 'bool', > + 'pbs-library-version': 'str' } } > + > + ## > diff --git a/debian/patches/series b/debian/patches/series > index 1ef7185..433efda 100644 > --- a/debian/patches/series > +++ b/debian/patches/series > @@ -59,3 +59,4 @@ pve/0055-PVE-Migrate-dirty-bitmap-state-via-savevm.patch > pve/0056-migration-block-dirty-bitmap-migrate-other-bitmaps-e.patch > pve/0057-PVE-fix-aborting-multiple-CREATED-jobs-in-sequential.patch > pve/0058-PVE-fall-back-to-open-iscsi-initiatorname.patch > +pve/0059-pbs-backup-add-masterkey.patch >