Re: [PATCH v2 2/9] qemu: Introduced VIR_MIGRATE_TPM_SHARED_STORAGE for TPM migration
On 10/14/22 11:28, Daniel P. Berrangé wrote:
On Thu, Oct 06, 2022 at 04:07:13PM +0200, Michal Prívozník wrote:
> On 10/6/22 15:47, Daniel P. Berrangé wrote:
>> On Wed, Oct 05, 2022 at 10:02:00AM -0400, Stefan Berger wrote:
>>> Introduced VIR_MIGRATE_TPM_SHARED_STORAGE for migrating a TPM across
>>> shared storage.
>>>
>>> At this point do not support this flag in 'virsh', yet.
>>>
>>> Signed-off-by: Stefan Berger <stefanb(a)linux.ibm.com>
>>> ---
>>> include/libvirt/libvirt-domain.h | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>> diff --git a/include/libvirt/libvirt-domain.h
b/include/libvirt/libvirt-domain.h
>>> index 8357aea797..110929039d 100644
>>> --- a/include/libvirt/libvirt-domain.h
>>> +++ b/include/libvirt/libvirt-domain.h
>>> @@ -1098,6 +1098,14 @@ typedef enum {
>>> * Since: 8.5.0
>>> */
>>> VIR_MIGRATE_ZEROCOPY = (1 << 20),
>>> +
>>> + /* Support TPM migration across hosts that have shared storage setup
for
>>> + * the directory structure holding the state of TPMs. Typically this
would
>>> + * mean that the directory /var/lib/libvirt/swtpm is shared.
>>> + *
>>> + * Since: 8.9.0
>>> + */
>>> + VIR_MIGRATE_TPM_SHARED_STORAGE = (1 << 21),
>>
>> Why do we need this flag at all. We don't require users to set any flag
>> when dealing with shared storage for disks, we just make sure we detect
>> shared storage and "do the right thing" with it.
>
> That could work. Until the state is stored on a shared FS but not shared
> across migration hosts. But I guess our disk migration code would fail
> then too, wouldn't it?
Exactly, if our existing code is good enough for disks for the last
NNN years, then its good enough for TPM too. If someone finds a broken
scenario, then we'll need to fix it for all cases, and that'll require
something more general than a VIR_MIGRATE_TPM_SHARED_STORAGE flag.
It is basically akin to a "make it work=yes" setting, and actually
introduces failure scenarios that would not otherwise exist. eg
if someone sets VIR_MIGRATE_TPM_SHARED_STORAGE when the TPM is on
local only storage, or fails to set it when using shared storage.
I was trying to find out how storage is being copied and whether there is any testing
going on regarding whether storage for storage is shared or not and how that's done.
However, it seems there's an explicit hint coming from the user encoded in the virsh
command line options about non-shared storage for storage. Or maybe I not dig deep
enough?
if (vshCommandOptBool(cmd, "copy-storage-all"))
flags |= VIR_MIGRATE_NON_SHARED_DISK;
if (vshCommandOptBool(cmd, "copy-storage-inc"))
flags |= VIR_MIGRATE_NON_SHARED_INC;
if (vshCommandOptBool(cmd, "copy-storage-synchronous-writes")) {
if (!(flags & VIR_MIGRATE_NON_SHARED_DISK) &&
!(flags & VIR_MIGRATE_NON_SHARED_INC)) {
vshError(ctl, "'--copy-storage-synchronous-writes' requires one
of '--copy-storage-all', 'copy-storage-inc'");
goto out;
}
flags |= VIR_MIGRATE_NON_SHARED_SYNCHRONOUS_WRITES;
}
So we won't do it this way for TPM state migration. Instead we can try to write on the
source migration side
a) a simple file and detect whether the file is at the destination
b) a file with either a name or content that only the source and destination libvirts
would know at this point
b) is to prevent stale files from becoming indicators for shared storage.
If you have any suggestions on how to do it or what data to write into the test file, let
me know. A hash over some shared knowledge would do the trick I guess.
Stefan
Ergo, we should not add this flag to our public API.
With regards,
Daniel