From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Stefan Reiter <s.reiter@proxmox.com>
Subject: Re: [pve-devel] [PATCH qemu] pbs: add master key support
Date: Wed, 10 Feb 2021 13:52:20 +0100 [thread overview]
Message-ID: <1612961453.pt5yfbqfko.astroid@nora.none> (raw)
In-Reply-To: <789a02a5-d5c4-b9d1-b8e8-e99569d0639b@proxmox.com>
On February 10, 2021 12:05 pm, Stefan Reiter wrote:
> 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.
no objections from my side. sorry for not converting before sending (I
used quilt to apply all patches)
>
> 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 <f.gruenbichler@proxmox.com>
>> ---
>>
>> 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
>>
>
next prev parent reply other threads:[~2021-02-10 12:52 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-08 13:08 [pve-devel] [PATCH-SERIES 0/4] PBS master key integration Fabian Grünbichler
2021-02-08 13:08 ` [pve-devel] [PATCH proxmox-backup-qemu] api: add master key support Fabian Grünbichler
2021-02-12 14:38 ` [pve-devel] applied: " Thomas Lamprecht
2021-02-08 13:08 ` [pve-devel] [PATCH qemu] pbs: " Fabian Grünbichler
2021-02-10 11:05 ` Stefan Reiter
2021-02-10 12:52 ` Fabian Grünbichler [this message]
2021-02-08 13:08 ` [pve-devel] [PATCH v2 storage] pbs: allow setting up a master key Fabian Grünbichler
2021-04-22 20:00 ` [pve-devel] applied: " Thomas Lamprecht
2021-02-08 13:08 ` [pve-devel] [PATCH qemu-server] vzdump: add master key support Fabian Grünbichler
2021-05-28 11:50 ` Thomas Lamprecht
2021-05-28 12:09 ` [pve-devel] [PATCH REBASE " Fabian Grünbichler
2021-06-02 14:51 ` [pve-devel] applied: " Thomas Lamprecht
2021-05-12 9:54 ` [pve-devel] [PATCH-SERIES 0/4] PBS master key integration Fabian Ebner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1612961453.pt5yfbqfko.astroid@nora.none \
--to=f.gruenbichler@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
--cc=s.reiter@proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox