[libvirt] [PATCH 19/21] qemu_hotplug: new function qemuDomainRemoveAuditDevice()
Laine Stump
laine at laine.org
Fri Mar 22 14:02:43 UTC 2019
On 3/22/19 8:24 AM, Peter Krempa wrote:
> On Thu, Mar 21, 2019 at 18:28:59 -0400, Laine Stump wrote:
>> This function can be called with a virDomainDevicePtr and whether or
>> not the removal was successful, and it will call the appropriate
>> virDomainAudit*() function with the appropriate args for whatever type
>> of device it's given (or do nothing, if that's appropriate). This
>> permits generalizing some code that currently has a separate copy for
>> each type of device.
>>
>> Signed-off-by: Laine Stump <laine at laine.org>
>> ---
>> src/qemu/qemu_hotplug.c | 72 +++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 72 insertions(+)
>>
>> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>> index b0e2c738b9..5e5ffe16d3 100644
>> --- a/src/qemu/qemu_hotplug.c
>> +++ b/src/qemu/qemu_hotplug.c
>> @@ -5208,6 +5208,78 @@ qemuDomainRemoveRedirdevDevice(virQEMUDriverPtr driver,
>> }
>>
>>
>> +static inline void
> Don't use inline, it's pointless.
I used inline to prevent the "static function defined but not used"
error from the compiler. It's removed in the next patch when I start
calling it.
>
>> +qemuDomainRemoveAuditDevice(virDomainObjPtr vm,
>> + virDomainDeviceDefPtr detach,
>> + bool success)
> @success is always false in the one call place the other patches add.
> Especially I doubt it will ever be different.
I made (and placed) the function with the assumption that in the future
the qemuDomainRemove*Device() functions might be refactored in a similar
manner (patch 21 is the first step in this, actually), and we would then
wnat to call this function from common code rather than each
type-specific function calling their own type-specific audit. (I even
considered making it *more* generic by sending attach/detach/update, but
decided that was a bit too ambitious).
> All async device deletion
> callbacks do their own per-device-type audit call on success.
in their qemuDomainRemove*Device() function, yes. But those functions
could also do with some refactoring.
>
> This function is unused thus breaks build.
Really? Adding "inline" fixed that for me (on Fedora 29 using gcc). In
the past I had taken care of this with ATTRIBUTE_SOMETHING, but I
couldn't remember what ATTRIBUTE it was, or the proper place in the
function header to put it, so I used inline and no longer had an error.
So what's the "proper" way to indicate that a static function isn't
currently being called? I'd rather keep its introduction in a separate
patch from the one where I start using it.
>
>> +{
>> + switch ((virDomainDeviceType)detach->type) {
>> + case VIR_DOMAIN_DEVICE_CHR:
>> + virDomainAuditChardev(vm, detach->data.chr, NULL, "detach", success);
>> + break;
>> +
>> + case VIR_DOMAIN_DEVICE_DISK:
>> + virDomainAuditDisk(vm, detach->data.disk->src, NULL, "detach", success);
>> + break;
>> +
>> + case VIR_DOMAIN_DEVICE_NET:
>> + virDomainAuditNet(vm, detach->data.net, NULL, "detach", success);
>> + break;
>> +
>> + case VIR_DOMAIN_DEVICE_HOSTDEV:
>> + virDomainAuditHostdev(vm, detach->data.hostdev, "detach", success);
>> + break;
>> +
>> + case VIR_DOMAIN_DEVICE_RNG:
>> + virDomainAuditRNG(vm, detach->data.rng, NULL, "detach", success);
>> + break;
>> +
>> + case VIR_DOMAIN_DEVICE_MEMORY: {
>> + unsigned long long oldmem = virDomainDefGetMemoryTotal(vm->def);
>> + unsigned long long newmem = oldmem - detach->data.memory->size;
>> +
>> + virDomainAuditMemory(vm, oldmem, newmem, "update", success);
> This is currently not audited, I think it should be added separately.
>
Actually, I think currently only disk, net, and hostdev are audited on a
*failure* to remove. According to a discussion with danpb on irc a
couple days ago, it's proper to audit failure to detach for any device
that would normally be audited on a successful detach, but I can remove
those that are currently unaudited on failure, and add them back in a
later patch.
More information about the libvir-list
mailing list