[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] qemu: Support chardevs with ARM virt machines



On Sun, Jun 25, 2017 at 4:07 AM, Andrea Bolognani <abologna redhat com> wrote:
> On Sat, 2017-06-24 at 16:07 +0200, Christoffer Dall wrote:
>> > The way I see it, the bug is about libvirt being unable to
>> > launch guests which use the <console><log> feature, and with
>> > that in mind your patch is correct but doesn't solve the
>> > issue, because even thought that specific error is gone you
>> > immediately run into a different one and your guest is still
>> > unable to start.
>>
>> I didn't experience this, it was actually working on my end.  I wonder
>> if it's related to the QEMU version, where I seem to remember we
>> changed what some serial options turned into.  But I for sure did not
>> see "-device isa-serial..." on the command line, so maybe not.
>
> That's very different from the behavior I'm seeing, and I
> can't figure out why that would be the case. That's why
> having your QEMU command line would be very useful.
>
> As for differences in QEMU binaries, there might be some
> capability that I haven't considered and influences the
> generated command line. I'll look into that.

Cool,  I'll have a look as well and will document my complete
environment, then hopefully we can diff with yours and see where this
ISA thing shows up.

>
>> In any case, I'll reproduce again when I'm back and send you the details.
>
> Sounds good to me.
>
>> > Just to be clear: I'm not against this patch, we definitely
>> > want to fix virQEMUCapsSupportsChardev(). What gave me pause
>> > is simply the fact that you seemed to claim it made the
>> > <console><log> feature usable, which I'm still unconvinced
>> > is actually the case.
>>
>> Oh, I didn't intend to claim that.  I intended to claim that
>>
>> <serial type='pty'>
>>     <log file='/tmp/testlogfile.log' append='off'/>
>> <target port='0'/>
>>
>> now works.
>
> Well, that's the same thing, really :)

I didn't know that.

>
> Adding <serial type='pty'/> will automatically add
> <console type='pty'/>, so if one works the other should
> work as well, since they translate to a single QEMU option
> at the end of the day.
>

Thanks for the explanation.

>> I'm not sure where I claimed more beyond that, can you
>> point me to specifics (this patch or the bug report, etc.) and I'll be
>> happy to correct that?
>
> https://bugs.linaro.org/show_bug.cgi?id=2777#c36
>
> Also, twice in the message I'm replying to ;)
>

Please forgive my libvirt ignorance, I didn't know that <serial> and
<console> would end up doing the same thing.

>> At this point I'm a little confused about how to proceed here.  Would
>> you like further evidence of an environment that reproduces the issue
>> with console and the isa bus, with additional logic added to this
>> patch to fix that, or should we get this patch merged and fix the
>> other issue separately?
>
> We can merge the patch without further changes to it, as
> it fixes part of the issues that prevent the feature to work.
>
> Actually, I just added
>
> Reviewed-by: Andrea Bolognani <abologna redhat com>
>
> and pushed it :)
>
>> I'll try to look at reproducing the isa bus thing and seeing if I can
>> come up with a fix when I'm back, unless someone beats me to it, which
>> is not unlikely given the time it takes me to dig through libvirt
>> abstraction layers.
>
> I thought that fixing this would require QEMU changes, but
> Drew recently pointed out[1] that it might be possible to
> make it work using existing QEMU features only. I'll look
> into that in the next few days.

Sounds good, and let us know if we can help on the QEMU side.

>
> Enjoy your vacation ^^

Thanks,
-Christoffer


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]