Re: [libvirt] [Qemu-devel] [PATCH] qdev: DEVICE_DELETED event
by Osier Yang
On 2013年03月08日 03:15, Michael S. Tsirkin wrote:
> On Thu, Mar 07, 2013 at 08:00:29PM +0100, Andreas Färber wrote:
>> Am 07.03.2013 19:12, schrieb Michael S. Tsirkin:
>>> On Thu, Mar 07, 2013 at 06:23:46PM +0100, Markus Armbruster wrote:
>>>> "Michael S. Tsirkin"<mst(a)redhat.com> writes:
>>>>
>>>>> On Thu, Mar 07, 2013 at 03:14:15PM +0100, Markus Armbruster wrote:
>>>>>> Andreas Färber<afaerber(a)suse.de> writes:
>>>>>>
>>>>>>> Am 07.03.2013 11:07, schrieb Michael S. Tsirkin:
>>>>>>>> On Thu, Mar 07, 2013 at 10:55:23AM +0100, Markus Armbruster wrote:
>>>>>>>>> "Michael S. Tsirkin"<mst(a)redhat.com> writes:
>>>>>>>>>
>>>>>>>>>> On Wed, Mar 06, 2013 at 02:57:22PM +0100, Andreas Färber wrote:
>>>>>>>>>>> Am 06.03.2013 14:00, schrieb Michael S. Tsirkin:
>>>>>>>>>>>> libvirt has a long-standing bug: when removing the device,
>>>>>>>>>>>> it can request removal but does not know when does the
>>>>>>>>>>>> removal complete. Add an event so we can fix this in a robust way.
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Michael S. Tsirkin<mst(a)redhat.com>
>>>>>>>>>>>
>>>>>>>>>>> Sounds like a good idea to me. :)
>>>>>>>>>>>
>>>>>>>>>>> [...]
>>>>>>>>>>>> diff --git a/hw/qdev.c b/hw/qdev.c
>>>>>>>>>>>> index 689cd54..f30d251 100644
>>>>>>>>>>>> --- a/hw/qdev.c
>>>>>>>>>>>> +++ b/hw/qdev.c
>>>>>>>>>>>> @@ -29,6 +29,7 @@
>>>>>>>>>>>> #include "sysemu/sysemu.h"
>>>>>>>>>>>> #include "qapi/error.h"
>>>>>>>>>>>> #include "qapi/visitor.h"
>>>>>>>>>>>> +#include "qapi/qmp/qjson.h"
>>>>>>>>>>>>
>>>>>>>>>>>> int qdev_hotplug = 0;
>>>>>>>>>>>> static bool qdev_hot_added = false;
>>>>>>>>>>>> @@ -267,6 +268,11 @@ void qdev_init_nofail(DeviceState *dev)
>>>>>>>>>>>> /* Unlink device from bus and free the structure. */
>>>>>>>>>>>> void qdev_free(DeviceState *dev)
>>>>>>>>>>>> {
>>>>>>>>>>>> + if (dev->id) {
>>>>>>>>>>>> + QObject *data = qobject_from_jsonf("{ 'device': %s }", dev->id);
>>>>>>>>>>>> + monitor_protocol_event(QEVENT_DEVICE_DELETED, data);
>>>>>>>>>>>> + qobject_decref(data);
>>>>>>>>>>>> + }
>>>>>>>>>>>> object_unparent(OBJECT(dev));
>>>>>>>>>>>> }
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I'm pretty sure this is the wrong place to fire the notification. We
>>>>>>>>>>> should rather do this when the device is actually deleted - which
>>>>>>>>>>> qdev_free() does *not* actually guarantee, as criticized in the s390x
>>>>>>>>>>> and unref'ing contexts.
>>>>>>>>>>> I would suggest to place your code into device_unparent() instead.
>>>>>>>>>>>
>>>>>>>>>>> Another thing to consider is what data to pass to the event: Not all
>>>>>>>>>>> devices have an ID.
>>>>>>>>>>
>>>>>>>>>> If they don't they were not created by management so management is
>>>>>>>>>> probably not interested in them being removed.
>>>>>>>>>>
>>>>>>>>>> We could always add a 'path' key later if this assumption
>>>>>>>>>> proves incorrect.
>>>>>>>>>
>>>>>>>>> In old qdev, ID was all we had, because paths were busted. Thus,
>>>>>>>>> management had no choice but use IDs.
>>>>>>>>>
>>>>>>>>> If I understand modern qdev correctly, we got a canonical path. Old
>>>>>>>>> APIs like device_del still accept only ID. Should new APIs still be
>>>>>>>>> designed that way? Or should they always accept / provide the canonical
>>>>>>>>> path, plus optional ID for convenience?
>>>>>>>>
>>>>>>>> What are advantages of exposing the path to users in this way?
>>>>>>
>>>>>> The path is the device's canonical name. Canonical means path:device is
>>>>>> 1:1. Path always works. Qdev ID only works when the user assigned one.
>>>>>>
>>>>>> Funny case: board creates a hot-pluggable device by default (thus no
>>>>>> qdev ID), guest ejects it, what do you put into the event? Your code
>>>>>> simply doesn't emit one.
>>>>>>
>>>>>> You could blame the user; after all he could've used -nodefaults, and
>>>>>> added the device himself, with an ID.
>>>>>>
>>>>>> I blame your design instead, which needlessly complicates the event's
>>>>>> semantics: it gets emitted only for devices with a qdev ID. Which you
>>>>>> neglected to document clearly, by the way.
>>>>>
>>>>> Good point, I'll document this.
>>>>>
>>>>>> If you put the path into the event, you can emit it always, which is
>>>>>> simpler. Feel free to throw in the qdev ID.
>>>>>
>>>>> I don't blame anyone. User not assigning an id is a clear indication
>>>>> that user does not care about the lifetime of this device.
>>>>>
>>>>>>>> Looks like maintainance hassle without real benefits?
>>>>>>
>>>>>> I can't see path being a greater maintenance hassle than ID.
>>>>>
>>>>> Sure, the less events we emit the less we need to support.
>>>>> You want to expose all kind of internal events,
>>>>> then management will come to depend on it and
>>>>> we'll have to maintain them forever.
>>>>
>>>> Misunderstanding. I'm *not* asking for more events. I'm asking for the
>>>> DEVICE_DELETED event to carry the device's canonical name: its QOM path.
>>>>
>>>>>>> Anthony had rejected earlier QOM patches by Paolo related to qdev id,
>>>>>>> saying it was deprecated in favor of those QOM paths.
>>>>>>
>>>>>> More reason to put the path into the event, not just the qdev ID.
>>>>>
>>>>> libvirt does not seems to want it there. We'll always be able to
>>>>> add info but will never be able to remove info, keep it minimal.
>>>>
>>>> Yes, adding members to an event is easy. Doesn't mean we should do it
>>>> just for the heck of it. If we don't need a member now, and we think
>>>> there's a chance we won't need in the future, then we probably shouldn't
>>>> add it now.
>>>>
>>>> I believe the chance of not needing the QOM path is effectively zero.
>>>>
>>>> Moreover, we'd add not just a member in this case, we'd add a *trigger*.
>>>>
>>>> Before: the event gets emitted only for devices with a qdev ID.
>>>>
>>>> After: the event gets emitted for all devices.
>>>>
>>>> I very much prefer the latter, because it's simpler.
>>>>
>>>> [...]
>>>
>>> I still don't see why it's useful for anyone. For now I hear from the
>>> libvirt guys that this patch does exactly what they need so I'll keep it
>>> simple. You are welcome to send a follow-up patch adding a path
>>> and more triggers, I won't object.
>>
>> Well, the libvirt guys have been told to poll using qom-list, which
>> needs the path, not an ID. Using it in both places would make it
>> symmetrical - that may qualify as useful.
>> (I'm not aware of any id -> path lookup QMP command.)
>>
>> Nontheless, you can retain my Reviewed-by on v4+ as long as the code in
>> hw/qdev.c doesn't change.
>>
>> Andreas
>
> I suggested retrying device_del, this has an advantage of working
> on more qemu version.
I'm wondering if it could be long time to wait for the device_del
completes (AFAIK from previous bugs, it can be, though it should be
fine for most of the cases). If it's too long, it will be a problem
for management, because it looks like hanging. We can have a timeout
for the device_del in libvirt, but the problem is the device_del
can be still in progress by qemu, which could cause the inconsistency.
Unless qemu has some command to cancel the device_del.
Osier
11 years, 9 months
Re: [libvirt] Consultation of the patch "util: Prepare helpers for unpriv_sgio setting"
by yuxh
On 03/13/2013 03:41 PM, Osier Yang wrote:
> On 2013年03月13日 15:38, Osier Yang wrote:
>> On 2013年03月13日 15:20, yuxh wrote:
>>> On 03/13/2013 11:33 AM, Osier Yang wrote:
>>>> On 2013年03月13日 11:04, yuxh wrote:
>>>>> To Osier Yang
>>>>>
>>>>> Sorry for disturbing you on the working time, but I really need your
>>>>> help.
>>>>> I met a problem when I do the virtualization test on the
>>>>> environment of
>>>>> (Host: RHEL6.4GA. Guest: RHEL6.4GA) and it seems to have relationship
>>>>> with your patch of "util: Prepare helpers for unpriv_sgio setting".
>>>>
>>>> Never mind, it's the bug I produced. :(
>>>>
>>>>>
>>>>> I added a disk to virtio-scsi bus through the VM's XML file like
>>>>> this:
>>>>> <disk type='block' device='disk'>
>>>>> <driver name='qemu' type='raw' io='threads'/>
>>>>> <source dev='/mnt/test.raw'/>
>>>>> <target dev='sdb' bus='scsi'/>
>>>>> <shareable/>
>>>>> </disk>
>>>>>
>>>>> Then when I started the VM, I met the error.
>>>>> [root@build SOURCES]# virsh start pan64ga
>>>>> error: Failed to start domain pan64ga
>>>>> error: Unable to get minor number of device '/mnt/test.raw': Invalid
>>>>> argument
>>>>>
>>>>> [root@build SOURCES]#
>>>>>
>>>>> I tried other source files and other way of using if and I found:
>>>>> 1). If we use the files which format is raw, qed and qcow2 as the
>>>>> disk
>>>>> this error occur again.
>>>>> 2). If we use partition(such as /dev/sdb1) or whole disk(such as
>>>>> /dev/sdb) this error doesn't occur.
>>>>> 3). If we delete the "<shareable/>" then files, partition and disk
>>>>> all
>>>>> can be used correctly.
>>>>>
>>>>> And I investigate the error msg. it is returned by
>>>>> qemuGetSharedDiskKey
>>>>> fuction.
>>>>> char *
>>>>> qemuGetSharedDiskKey(const char *disk_path)
>>>>> {
>>>>> ...
>>>>> // error occur here. return value is -22
>>>>> if ((rc = virGetDeviceID(disk_path, &maj, &min)) < 0) {
>>>>> virReportSystemError(-rc,
>>>>> _("Unable to get minor number of device '%s'"),
>>>>> disk_path);
>>>>> return NULL;
>>>>> }
>>>>> ...
>>>>> }
>>>>>
>>>>> Go on the virGetDeviceID fuction:
>>>>> int
>>>>> virGetDeviceID(const char *path, int *maj, int *min)
>>>>> {
>>>>> ...
>>>>>
>>>>> if (!S_ISBLK(sb.st_mode)) {
>>>>> VIR_FREE(canonical_path);
>>>>> return -EINVAL; // error occur here and it return -22
>>>>> }
>>>>> ...
>>>>> }
>>>>>
>>>>> So the error occur place is "S_ISBLK(sb.st_mode)" and I tried
>>>>> compiled
>>>>> the libvirt with these lines deleted and the error didn't happen
>>>>> again.
>>>>> Also I checked the disk if VM and it worked well.
>>>>> I found this fuction virGetDeviceID is come from your patch of "util:
>>>>> Prepare helpers for unpriv_sgio setting". But I am not familiar with
>>>>> libvirt.
>>>>> So I have to consult with you about whether this "S_ISBLK" check is
>>>>> vital and If we can delete it or
>>>>> find out some method to avoid this error so we can use the file as
>>>>> shared disk in VM.
>>>>
>>> Sorry, I have tried the the libvirt's upstream code and it still have
>>> this error.
>>> According to the code, if we use "<disk type='block' device='disk'> "
>>> and "<shareable/> " to set the disk's configuration in VM's XML
>>> file, it
>>> will go through like below:
>>>
>>> int
>>> qemuAddSharedDisk(virQEMUDriverPtr driver,
>>> virDomainDiskDefPtr disk,
>>> const char *name)
>>> { ...
>>> if (!(key = qemuGetSharedDiskKey(disk->src)))
>>> goto cleanup;
>>> ...
>>> }
>>>
>>> char *
>>> qemuGetSharedDiskKey(const char *disk_path)
>>> {
>>> ...
>>> if ((rc = virGetDeviceID(disk_path, &maj, &min)) < 0) {
>>> virReportSystemError(-rc,
>>> _("Unable to get minor number of device '%s'"),
>>> disk_path);
>>> return NULL;
>>> }
>>> ...
>>> }
>>>
>>> int
>>> virGetDeviceID(const char *path, int *maj, int *min)
>>> {
>>> struct stat sb;
>>>
>>> if (stat(path, &sb) < 0)
>>> return -errno;
>>>
>>> if (!S_ISBLK(sb.st_mode))
>>> return -EINVAL;
>>>
>>> if (maj)
>>> *maj = major(sb.st_rdev);
>>> if (min)
>>> *min = minor(sb.st_rdev);
>>>
>>> return 0;
>>> }
>>>
>>> To make a long story short, the virGetDeviceID fuction want to get the
>>> maj and min number of the source file which we provided as the VM's
>>> disk. But the file such as /mnt/test.raw doesn't have the maj and min
>>> number.
>>> So virGetDeviceID return " -EINVAL" and qemuGetSharedDiskKey print the
>>> error message of "Unable to get minor number of device /mnt/test.raw".
>>> That's the reason the VM can't be started.
>>> So I just want to know now, if the way I used a file to act as VM's
>>> disk
>>> and set "<disk type='block' device='disk'>" , "<shareable/> " at the
>>> same time is reasonable.
>>
>> Hum, sorry I didn't read your problem carefully. But...
>>
>> It's not reasonable. As the disk source you use is /mnt/test.raw, I
>> believe it's not a block device. However, you used disk type='block'.
>> Libvirt honors what you specified, and tries to grab the device IDs
>> of the disk source (it expects the disk source is block, but it's not).
>>
>> Regards,
>> Osier
Thank you for your explanation.
But I can use the disk source of /mnt/test.raw when disk type='block'
when the "<shareable/> " is not set. And the error come only when
"<shareable/> " is set at the same time.
So if when type='block' is set, the disk source mush be block device, I
think there need to add some
code in high level to implement this restrict not only when
"<shareable/> " is set.
Thanks
yuxh
>> --
>> libvir-list mailing list
>> libvir-list(a)redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
>
>
11 years, 9 months
[libvirt] One problem of libvirt for fc18.
by Li Zhang
Hi all,
We are trying to compile libvirt on fc18.
But it always reports such a problem with rpcgen.
"make" fails:
GEN lxc/lxc_monitor_protocol.h
/usr/bin/rpcgen: C preprocessor failed with exit code 1
cannot shutdown /usr/bin/rpcgen: at ./rpc/genprotocol.pl line 124.
make[2]: *** [lxc/lxc_monitor_protocol.h] Error 1
This should be the problem when executing rpcgen on fc18.
[root@xxx libvirt]$ rpcgen --version
rpcgen (GNU libc) 2.16
Any idea on how to resolve it or do a workaround?
Thanks a lot. :)
--Li
11 years, 9 months
[libvirt] [PATCH v3 0/4] qemu: -dtb option support
by Olivia Yin
Sometime QEMU need load specific device tree binary images.
These patches provide -dtb option support and update docs/tests.
Olivia Yin (4):
QEMU: add -dtb option support
qemu: add dtb capability
docs: add -dtb option support to QEMU
tests: add dtb test case
src/conf/domain_conf.c | 4 ++++
src/conf/domain_conf.h | 1 +
src/qemu/qemu_command.c | 6 ++++++
src/qemu/qemu_capabilities.c | 3 +++
src/qemu/qemu_capabilities.h | 1 +
docs/formatdomain.html.in | 5 +++++
docs/schemas/domaincommon.rng | 5 +++++
tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.args | 1 +
tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.xml | 28 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 2 +
11 years, 9 months
[libvirt] [PATCH 1/1] Set legacy USB option with default for ppc64.
by Li Zhang
From: Li Zhang <zhlcindy(a)linux.vnet.ibm.com>
Currently, -device xxx still can't work well for ppc64 platform.
It's better use legacy USB option with default for ppc64.
This patch is to legacy USB option with default for ppc64.
Signed-off-by: Li Zhang <zhlcindy(a)linux.vnet.ibm.com>
---
src/qemu/qemu_command.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1c9bfc9..618dfb1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5783,7 +5783,8 @@ qemuBuildCommandLine(virConnectPtr conn,
}
} else if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
cont->model == -1 &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI)) {
+ (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI) ||
+ def->os.arch == VIR_ARCH_PPC64)) {
if (usblegacy) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Multiple legacy USB controllers are "
--
1.7.10.1
11 years, 9 months
[libvirt] libvirt accidentally destroys guests after being restarted
by Andrey Korolyov
Sorry in advance for possible top-post, I`m not able to add proper
messageid here.
>Does it ever occur if you don't run with DHCP snooping enabled?
>
> Stefan
No, please disregard those errors. We don`t run DHCP snooping/IP
learning on interfaces but only modified clean-traffic rules, current
issue almost for sure does not have relation to it.
11 years, 9 months
[libvirt] RFC: vfio pci passthrough support
by Laine Stump
VFIO is a new method of doing PCI device assignment ("PCI passthrough"
aka "<hostdev>") available in newish kernels (3.6?; it's in Fedora 18 at
any rate) and via the "vfio-pci" device in qemu-1.4+. In contrast to the
traditional KVM PCI device assignment (available via the "pci-assign"
device in qemu), VFIO works properly on systems using UEFI "Secure
Boot"; it also offers other advantages, such as grouping of related
devices that must all be assigned to the same guest (or not at all).
Here's some useful reading on the subject.
http://lwn.net/Articles/474088/
http://lwn.net/Articles/509153/
Short description (from Alex Williamson's KVM Forum Presentation)
1) Assume this is the device you want to assign:
01:10.0 Ethernet controller: Intel Corporation 82576
Virtual Function (rev 01)
2) Find the vfio group of this device
:
# readlink /sys/bus/pci/devices/0000:01:10.0/iommu_group
../../../../kernel/iommu_groups/15
==> IOMMU Group = 15
3) Check the devices in the group:
# ls /sys/bus/pci/devices/0000:01:10.0/iommu_group/devices/
0000:01:10.0
(so this group has only 1 device)
4) Unbind from device driver
# echo 0000:01:10.0 >/sys/bus/pci/devices/0000:01:10.0/driver/unbind
5) Find vendor & device ID
$ lspci -n -s 01:10.0
01:10.0 0200: 8086:10ca (rev 01)
6) Bind to vfio-pci
$ echo 8086 10ca > /sys/bus/pci/drivers/vfio-pci/new_id
(this will result in a new device node "/dev/vfio/15", which is what qemu will use to setup the device for passthrough)
7) chown the device node so it is accessible by qemu user:
# chown qemu /dev/vfio/15; chgrp qemu /dev/vfio/15
(note that /dev/vfio/vfio, which is installed as 0600 root:root, must also be made mode 0666, still owned by root - this is supposedly not dangerous)
8) set the limit for locked memory equal to all of guest memory size + [some amount large enough to encompass all of io space]
# ulimit -l 2621440 # ((2048 + 512) * 1024)
9) pass to qemu using -device vfio-pci:
sudo qemu qemu-system-x86_64 -m 2048 -hda rhel6vm \
-vga std -vnc :0 -net none \
-enable-kvm \
-device vfio-pci,host=01:10.0,id=net0
(qemu will then use something like step (2) to figure out which device node it needs to use)
Why the "ulimit -l"?
--------------------
Any qemu guest that is using the old pci-assign must have *all* guest memory and IO space locked in memory. Normally the maximum amount of locked memory allowed for a process is controlled by "ulimit -l", but in the case of pc-assign, the kvm kernel module has always just ignored the -l limit and locked it all anyway.
With vfio-pci, all guest memory and IO space must still be locked in memory, but the vfio module *doesn't* ignore the process limits, so libvirt will need to set ulimit -l for any guest that wants to do vfio-based pci passthrough. Since (due to the possibility of hotplug) we don't know at the time the qemu process is started whether or not it might need to do a pci passthrough, we will need to use prlimit(2) to modify the limit of the already-running qemu.
Proposed XML Changes
--------------------
To support vfio pci device assignment in libvirt, I'm thinking something
like this (note that the <driver> subelement is already used for
<interface> and <disk> to choose which backend to use for a particular
device):
<hostdev managed='yes'>
<driver name='vfio'/>
...
</hostdev>
<interface type='hostdev' managed='yes'>
<driver name='vfio'/>
...
</hostdev>
(this new use of <driver> inside <interface> wouldn't conflict with the existing <driver name='qemu|vhost'>, since neither of those could ever possibly be a valid choice for <interface type='hostdev'>. The one possible problem would be if someone had an <interface type='network'> which might possibly point to a hostdev or standard bridged network, and wanted to make sure that in the case of a bridged network, that <driver name='qemu' was used. I suppose in this case, the driver name in the network definition would override any driver name in the interface?)
Sepaking of <network>, here's how vfio would be specified in a hostdev <network> definition:
<network>
<name>vfio-net</name>
<forward mode='hostdev' managed='yes'>
<driver name='vfio'/>
<pf dev='eth3'/> <!-- or a list of VFs -->
</forward>
...
</network>
Another possibility for the <network> xml would be to add a "driver='vfio'" to each individual <interface> line, in case someone wanted some devices in a pool to be asigned using vfio and some using the old style, but that seems highly unlikely (and could create problems in the future if we ever needed to add a 2nd attribute to the <driver> element).
Actually, at one point I considered that vfio should be turned on globally in libvirtd.conf (or qemu.conf), but that would make switchover a tedious process, as all existing guests using PCI passthrough would need to be shutdown prior to the change. As long as there are no technical problems with allowing both types on the same host, it's more flexible to choose on a device-by-device basis.
Now some questions:
1) Is there any reason that we shouldn't/can't allow both pci-assign and vfio-pci at the same time on the same host (and even guest).
2) Does it make any sense to support a "managed='no'" mode for vfio, which skipped steps 2-6 above? (this would be parallel to the existing pci-assign managed='no'(where no unbinding/binding of the device to the host's pci-stub driver is done, but the device name is simply passed to qemu assuming that all that work was already done)) Or should <driver name='vfio'/> automatically mean that all unbinding/binding be done for each device.
3) Is it at all bothersome that qemu must be the one opening the device node, and that there is apparently no way to have libvirt open it and send the fd to qemu?
11 years, 9 months
[libvirt] [PATCHv2] audit: Audit resources used by VirtIO RNG
by Peter Krempa
This patch adds auditing of resources used by Virtio RNG devices. Only
resources on the local filesystems are audited properly. For remote
character device sources "remote" is logged as the path and "none"
is logged for character device backends that don't have a source.
The audit logs look like:
For the 'random' backend:
type=VIRT_RESOURCE msg=audit(1363099126.643:31): pid=995252 uid=0 auid=4294967295 ses=4294967295 msg='virt=kvm resrc=rng reason=start vm="qcow-test" uuid=118733ed-b658-3e22-a2cb-4fe5cb3ddf79 old-rng="?" new-rng="/dev/random": exe="/home/pipo/libvirt/daemon/.libs/libvirtd" hostname=? addr=? terminal=pts/0 res=success'
For local character device source:
type=VIRT_RESOURCE msg=audit(1363100164.240:96): pid=995252 uid=0 auid=4294967295 ses=4294967295 msg='virt=kvm resrc=rng reason=start vm="qcow-test" uuid=118733ed-b658-3e22-a2cb-4fe5cb3ddf79 old-rng="?" new-rng="/tmp/unix.sock": exe="/home/pipo/libvirt/daemon/.libs/libvirtd" hostname=? addr=? terminal=pts/0 res=success'
For a remote character device source:
type=VIRT_RESOURCE msg=audit(1363100291.450:127): pid=995252 uid=0 auid=4294967295 ses=4294967295 msg='virt=kvm resrc=rng reason=start vm="qcow-test" uuid=118733ed-b658-3e22-a2cb-4fe5cb3ddf79 old-rng="?" new-rng="remote": exe="/home/pipo/libvirt/daemon/.libs/libvirtd" hostname=? addr=? terminal=pts/0 res=success'
And for character backends that don't have a source:
type=VIRT_RESOURCE msg=audit(1363100520.513:158): pid=995252 uid=0 auid=4294967295 ses=4294967295 msg='virt=kvm resrc=rng reason=start vm="qcow-test" uuid=118733ed-b658-3e22-a2cb-4fe5cb3ddf79 old-rng="?" new-rng="none": exe="/home/pipo/libvirt/daemon/.libs/libvirtd" hostname=? addr=? terminal=pts/0 res=success'
---
Version 2:
- audit also local character devices
- audit implicit usage of '/dev/random'
- CC'd Steven to verify the format of the audit messages.
src/conf/domain_audit.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 117 insertions(+)
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 8cd522a..0f2cf4f 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -57,6 +57,37 @@ virDomainAuditGetRdev(const char *path ATTRIBUTE_UNUSED)
}
#endif
+static const char *
+virDomainAuditChardev(virDomainChrSourceDefPtr chr)
+{
+ if (!chr)
+ return "?";
+
+ switch ((enum virDomainChrType) chr->type) {
+ case VIR_DOMAIN_CHR_TYPE_PTY:
+ case VIR_DOMAIN_CHR_TYPE_DEV:
+ case VIR_DOMAIN_CHR_TYPE_FILE:
+ case VIR_DOMAIN_CHR_TYPE_PIPE:
+ return chr->data.file.path;
+
+ case VIR_DOMAIN_CHR_TYPE_UNIX:
+ return chr->data.nix.path;
+
+ case VIR_DOMAIN_CHR_TYPE_TCP:
+ case VIR_DOMAIN_CHR_TYPE_UDP:
+ return "remote";
+
+ case VIR_DOMAIN_CHR_TYPE_NULL:
+ case VIR_DOMAIN_CHR_TYPE_VC:
+ case VIR_DOMAIN_CHR_TYPE_STDIO:
+ case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
+ case VIR_DOMAIN_CHR_TYPE_LAST:
+ return "none";
+ }
+
+ return "?";
+}
+
void
virDomainAuditDisk(virDomainObjPtr vm,
const char *oldDef, const char *newDef,
@@ -100,6 +131,89 @@ cleanup:
}
+static void
+virDomainAuditRNG(virDomainObjPtr vm,
+ virDomainRNGDefPtr newDef, virDomainRNGDefPtr oldDef,
+ const char *reason, bool success)
+{
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ char *vmname;
+ const char *newsrcpath = NULL;
+ const char *oldsrcpath = NULL;
+ char *oldsrc = NULL;
+ char *newsrc = NULL;
+ const char *virt;
+
+ virUUIDFormat(vm->def->uuid, uuidstr);
+ if (!(vmname = virAuditEncode("vm", vm->def->name)))
+ goto no_memory;
+
+ if (!(virt = virDomainVirtTypeToString(vm->def->virtType))) {
+ VIR_WARN("Unexpected virt type %d while encoding audit message",
+ vm->def->virtType);
+ virt = "?";
+ }
+
+ if (newDef) {
+ switch ((enum virDomainRNGBackend) newDef->backend) {
+ case VIR_DOMAIN_RNG_BACKEND_RANDOM:
+ if (newDef->source.file)
+ newsrcpath = newDef->source.file;
+ else
+ newsrcpath = "/dev/random";
+ break;
+
+ case VIR_DOMAIN_RNG_BACKEND_EGD:
+ newsrcpath = virDomainAuditChardev(newDef->source.chardev);
+ break;
+
+ case VIR_DOMAIN_RNG_BACKEND_LAST:
+ break;
+ }
+ }
+
+ if (!(newsrc = virAuditEncode("new-rng", VIR_AUDIT_STR(newsrcpath))))
+ goto no_memory;
+
+
+ if (oldDef) {
+ switch ((enum virDomainRNGBackend) oldDef->backend) {
+ case VIR_DOMAIN_RNG_BACKEND_RANDOM:
+ if (oldDef->source.file)
+ oldsrcpath = oldDef->source.file;
+ else
+ oldsrcpath = "/dev/random";
+ break;
+
+ case VIR_DOMAIN_RNG_BACKEND_EGD:
+ oldsrcpath = virDomainAuditChardev(oldDef->source.chardev);
+ break;
+
+ case VIR_DOMAIN_RNG_BACKEND_LAST:
+ break;
+ }
+ }
+
+ if (!(oldsrc = virAuditEncode("old-rng", VIR_AUDIT_STR(oldsrcpath))))
+ goto no_memory;
+
+ VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
+ "virt=%s resrc=rng reason=%s %s uuid=%s %s %s",
+ virt, reason, vmname, uuidstr,
+ oldsrc, newsrc);
+
+cleanup:
+ VIR_FREE(vmname);
+ VIR_FREE(oldsrc);
+ VIR_FREE(newsrc);
+ return;
+
+no_memory:
+ VIR_WARN("OOM while encoding audit message");
+ goto cleanup;
+}
+
+
void
virDomainAuditFS(virDomainObjPtr vm,
virDomainFSDefPtr oldDef, virDomainFSDefPtr newDef,
@@ -641,6 +755,9 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success)
virDomainAuditRedirdev(vm, redirdev, "start", true);
}
+ if (vm->def->rng)
+ virDomainAuditRNG(vm, vm->def->rng, NULL, "start", true);
+
virDomainAuditMemory(vm, 0, vm->def->mem.cur_balloon, "start", true);
virDomainAuditVcpu(vm, 0, vm->def->vcpus, "start", true);
--
1.8.1.5
11 years, 9 months
[libvirt] [PATCH] Fix missing error dispatch in virDomainLxcEnterNamespace
by Daniel P. Berrange
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The virDomainLxcEnterNamespace method mistakenly uses
virCheckFlags, which returns immediately instead of
virCheckFlagsGoto which jumps to the error cleanup
patch where there is a virDispatchError call
---
src/libvirt-lxc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/libvirt-lxc.c b/src/libvirt-lxc.c
index c1bf168..abbbc12 100644
--- a/src/libvirt-lxc.c
+++ b/src/libvirt-lxc.c
@@ -138,7 +138,7 @@ virDomainLxcEnterNamespace(virDomainPtr domain,
{
int i;
- virCheckFlags(0, -1);
+ virCheckFlagsGoto(0, error);
if (noldfdlist && oldfdlist) {
size_t nfds;
--
1.8.1.4
11 years, 9 months
