Re: [Qemu-devel] libvirt fails to start guest with latest code checkins

[Anthony Liguori <anthony@codemonkey.ws>]

Laszlo Ersek <lersek@redhat.com> writes:

> On 02/01/13 23:41, Bruce Rogers wrote:
>
>> Here is the invocation by libvirt, as recorded in /var/log/libvirt/qemu/<vmname>:
>> 2013-02-01 21:35:45.597+0000: starting up
>> LC_ALL=C
>> PATH=/sbin:/usr/sbin:/usr/local/sbin:/root/bin:/usr/local/bin:/usr/bin:
>> /bin:/usr/bin/X11:/usr/X11R6/bin:/usr/games:/usr/lib/mit/bin:/usr/lib/mit/sbin
>> HOME=/root USER=root LOGNAME=root TMPDIR=/tmp QEMU_AUDIO_DRV=none
>> /usr/bin/qemu-kvm -name sles11 -S -M pc -cpu
>> core2duo,+lahf_lm,+pdcm,+xtpr,+cx16,+tm2,+est,+vmx,+ds_cpl,+dtes64,+pbe,+tm,+ht,+ss,+acpi,+ds
>> -enable-kvm -m 512 -smp 2,sockets=2,cores=1,threads=1 -uuid
>> 36975709-16ae-85e0-c90f-09eb4ec7dec3 -no-user-config -nodefaults
>> -chardev
>> socket,id=charmonitor,path=/var/lib/libvirt/qemu/sles11.monitor,server,nowait
>> -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc
>> -no-shutdown -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2
>> -drive
>> file=/var/lib/kvm/images/sles11/disk0.raw,if=none,id=drive-virtio-disk0,format=raw
>> -device
>> virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
>> -drive if=none,id=drive-ide0-0-0,readonly=on,format=raw -device
>> ide-cd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 -netdev
>> tap,fd=18,id=hostnet0 -device
>> virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:59:31:da,bus=pci.0,addr=0x3
>> -vnc 127.0.0.1:0 -vga cirrus -device
>> virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5
>> WARNING: failed to find acpi-dsdt.aml
>> qemu-kvm: -device
>> virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:59:31:da,bus=pci.0,addr=0x3:
>> Property 'virtio-net-pci.netdev' can't find value 'hostnet0'
>> 2013-02-01 21:35:46.491+0000: shutting down
>
> (The "WARNING: failed to find acpi-dsdt.aml" message is fixed by commit 7e973bb2.)
>
> I think the following could be happening.
>
>     -netdev tap,fd=18,id=hostnet0
>
> is specified on the command line (generated by libvirt). Accordingly,
> the net_init_tap() function is entered with name="hostnet0".
>
> Looking at the tree at 264986e2^ (= e5dc0b4):
> - "tap->has_fd" is true,
> - fd / vnet_hdr / model are set up in that branch,
> - the net_init_tap() tail-calls net_init_tap_one(), with the configured
>   local variables, including "name" (input param):
>
>     return net_init_tap_one(tap, peer, model, name, ifname, script,
>                     downscript, tap->has_vhostfd ? tap->vhostfd : NULL,
>                     vnet_hdr, fd);
>
> Commit 264986e2, among other things, instantiates the net_init_tap_one()
> call for each branch of net_init_tap(), open-coding the parameters at
> each call site that originally used to be "buffered up" for the common
> final call. Unfortunately, there's a typo in the "tap->has_fd" branch:
>
>         if (net_init_tap_one(tap, peer, "tap", NULL, NULL,
>                              script, downscript,
>                              vhostfdname, vnet_hdr, fd)) {
>             return -1;
>         }
>
> The "name" parameter, which would ultimately go to
> qemu_net_client_setup(), is lost, hence assign_name() is used instead.
>
> The rest of the arguments seems OK. Suggested patch:

Indeed, looks like we came to the same conclusion here :-)

Regards,

Anthony Liguori

>
> ----[cut here]----
> diff --git a/net/tap.c b/net/tap.c
> index 1bf7609..48c254e 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -711,7 +711,7 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
>  
>          vnet_hdr = tap_probe_vnet_hdr(fd);
>  
> -        if (net_init_tap_one(tap, peer, "tap", NULL, NULL,
> +        if (net_init_tap_one(tap, peer, "tap", name, NULL,
>                               script, downscript,
>                               vhostfdname, vnet_hdr, fd)) {
>              return -1;
> ----[cut here]----
>
> Laszlo