On 7/2/24 16:25, John Levon wrote:
> On Tue, Jul 02, 2024 at 04:07:48PM +0200, Michal Prívozník wrote:
>
>>> + // TODO: support these here once tested.
>>> + case VIR_DOMAIN_DEVICE_GRAPHICS:
>>> + case VIR_DOMAIN_DEVICE_NET:
>>> + case VIR_DOMAIN_DEVICE_MEMORY:
>>> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
>>> + _("persistent update of device '%1$s' is not supported by test driver"),
>>> + virDomainDeviceTypeToString(dev->type));
>>> + return -1;
>>> +
>>
>> It's perfectly okay if ...
>>
>>> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
>>> + _("persistent update of device '%1$s' is not supported"),
>>> + virDomainDeviceTypeToString(dev->type));
>>
>> .. this error is reported instead.
>
> OK; I just thought it might be useful to separate out "we should add this" from
> "we don't support this ever".
I can see us having implementation for virtually all devices.
>
>>> + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
>>> + VIR_DOMAIN_AFFECT_CONFIG |
>>> + VIR_DOMAIN_DEVICE_MODIFY_FORCE, -1);
>>
>> LIVE is not supported and thus shouldn't be in list of supported flags.
>
> I was a bit unclear on the test hypervisor case - in a sense everything is both
> LIVE *and* CONFIG :)
I see what you mean, but no. The fact that libvirt doesn't really store
XMLs on disk for inactive domains doesn't necessarily mean there's no
distinction. The aim of test driver is to mimic real life scenarios as
closely as possible, so that when somebody is developing an app on top
of libvirt they can use the test driver instead of spawning real VMs and
thus test their app quickly. Therefore, the distinction between inactive
and live XMLs must be preserved.
>
>> I'm squashing in necessary changes and merging.
>
> Thanks!
You're welcome.
Michal
