The function to check if -chardev is supported by QEMU was written a
long time ago, where adding chardevs did not make sense on the fixed ARM
platforms.  Since then, we now have a general purpose virt platform,
which should support plugging in any device over PCIe which is supported
in a similar fashion on x86.

Signed-off-by: Christoffer Dall <cdall@linaro.org>
---
 src/qemu/qemu_capabilities.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 7f22492..1348af7 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5507,6 +5507,11 @@ virQEMUCapsSupportsChardev(const virDomainDef *def,
     if ((def->os.arch != VIR_ARCH_ARMV7L) && (def->os.arch != VIR_ARCH_AARCH64))
         return true;
 
+    /* The virt machine has a PCIe bus and allows plugging in the same type of
+     * devices as x86 systems do on a PCIe bus. */
+    if (qemuDomainIsVirt(def))
+        return true;
+
     /* This may not be true for all ARM machine types, but at least
      * the only supported non-virtio serial devices of vexpress and versatile
      * don't have the -chardev property wired up. */
-- 
2.9.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Wed, Jun 7, 2017 at 11:13 PM, Christoffer Dall <cdall@linaro.org> wrote:
> The function to check if -chardev is supported by QEMU was written a
> long time ago, where adding chardevs did not make sense on the fixed ARM
> platforms.  Since then, we now have a general purpose virt platform,
> which should support plugging in any device over PCIe which is supported
> in a similar fashion on x86.
>
> Signed-off-by: Christoffer Dall <cdall@linaro.org>
> ---
>  src/qemu/qemu_capabilities.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 7f22492..1348af7 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -5507,6 +5507,11 @@ virQEMUCapsSupportsChardev(const virDomainDef *def,
>      if ((def->os.arch != VIR_ARCH_ARMV7L) && (def->os.arch != VIR_ARCH_AARCH64))
>          return true;
>
> +    /* The virt machine has a PCIe bus and allows plugging in the same type of
> +     * devices as x86 systems do on a PCIe bus. */
> +    if (qemuDomainIsVirt(def))
> +        return true;
> +
>      /* This may not be true for all ARM machine types, but at least
>       * the only supported non-virtio serial devices of vexpress and versatile
>       * don't have the -chardev property wired up. */
> --
> 2.9.0
>


ping?

Thanks,
-Christoffer

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Wed, 2017-06-07 at 23:13 +0200, Christoffer Dall wrote:
> The function to check if -chardev is supported by QEMU was written a
> long time ago, where adding chardevs did not make sense on the fixed ARM
> platforms.  Since then, we now have a general purpose virt platform,
> which should support plugging in any device over PCIe which is supported
> in a similar fashion on x86.
> 
> Signed-off-by: Christoffer Dall <cdall@linaro.org>
> ---
>  src/qemu/qemu_capabilities.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 7f22492..1348af7 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -5507,6 +5507,11 @@ virQEMUCapsSupportsChardev(const virDomainDef *def,
>      if ((def->os.arch != VIR_ARCH_ARMV7L) && (def->os.arch != VIR_ARCH_AARCH64))
>          return true;
>  
> +    /* The virt machine has a PCIe bus and allows plugging in the same type of
> +     * devices as x86 systems do on a PCIe bus. */
> +    if (qemuDomainIsVirt(def))
> +        return true;
> +
>      /* This may not be true for all ARM machine types, but at least
>       * the only supported non-virtio serial devices of vexpress and versatile
>       * don't have the -chardev property wired up. */

We have two bugs tracking this issue:

  https://bugs.linaro.org/show_bug.cgi?id=2777
  https://bugzilla.redhat.com/show_bug.cgi?id=1435681

You mention in [1] that applying this patch and using a
recent QEMU fixes the problem for you, however I can't
say the same: I still get

  -device isa-serial,chardev=charserial0,id=serial0:
  No 'ISA' bus found for device 'isa-serial'

Would you mind sharing your guest XML and the resulting
QEMU command line?


[1] https://bugs.linaro.org/show_bug.cgi?id=2777#c36
-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Thu, Jun 22, 2017 at 8:30 AM, Andrea Bolognani <abologna@redhat.com> wrote:
> On Wed, 2017-06-07 at 23:13 +0200, Christoffer Dall wrote:
>> The function to check if -chardev is supported by QEMU was written a
>> long time ago, where adding chardevs did not make sense on the fixed ARM
>> platforms.  Since then, we now have a general purpose virt platform,
>> which should support plugging in any device over PCIe which is supported
>> in a similar fashion on x86.
>>
>> Signed-off-by: Christoffer Dall <cdall@linaro.org>
>> ---
>>  src/qemu/qemu_capabilities.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>> index 7f22492..1348af7 100644
>> --- a/src/qemu/qemu_capabilities.c
>> +++ b/src/qemu/qemu_capabilities.c
>> @@ -5507,6 +5507,11 @@ virQEMUCapsSupportsChardev(const virDomainDef *def,
>>      if ((def->os.arch != VIR_ARCH_ARMV7L) && (def->os.arch != VIR_ARCH_AARCH64))
>>          return true;
>>
>> +    /* The virt machine has a PCIe bus and allows plugging in the same type of
>> +     * devices as x86 systems do on a PCIe bus. */
>> +    if (qemuDomainIsVirt(def))
>> +        return true;
>> +
>>      /* This may not be true for all ARM machine types, but at least
>>       * the only supported non-virtio serial devices of vexpress and versatile
>>       * don't have the -chardev property wired up. */
>
> We have two bugs tracking this issue:
>
>   https://bugs.linaro.org/show_bug.cgi?id=2777
>   https://bugzilla.redhat.com/show_bug.cgi?id=1435681
>
> You mention in [1] that applying this patch and using a
> recent QEMU fixes the problem for you, however I can't
> say the same: I still get
>
>   -device isa-serial,chardev=charserial0,id=serial0:
>   No 'ISA' bus found for device 'isa-serial'
>

Well that's not the bug reported in 2777.  If you try to create an ISA
bus or configure your domain with an ISA bus on AArch64 your are bound
to fail, because we never had, and we never will have, support for an
ISA bus on AArch64.

To verify what this patch changes, you can use the test xml file
listed in [1] as well:

<domain type='kvm'>
  <name>testlogfile</name>
  <memory unit='KiB'>524288</memory>
  <os>
    <type arch='aarch64' machine='virt-2.7'>hvm</type>
  </os>
  <devices>
    <serial type='pty'>
      <log file='/tmp/testlogfile.log' append='off'/>
      <target port='0'/>
    </serial>
  </devices>
</domain>

Or any working domain configuration where you add <log file='...' />
to the domain definition.

It may be that we have an additional bug in libvirt that it under some
circumstances tries to create an ISA bus with an AArch64 VM, but I
don't see that being related to the patch above?

Note that the submitted patch fixes virQEMUCapsSupportsChardev, which
should be independent from any ISA bus fixes in libvirt, but given my
very limited experience with libvirt, I may be wrong here.

In summary, if your test setup goes from "error: unsupported
configuration: logfile not supported in this QEMU binary" to "-device
isa-serial,chardev=charserial0,id=serial0: No 'ISA' bus found for
device 'isa-serial'" then I'd argue that my patch solves the first
issue.

Thanks,
-Christoffer


[1] https://bugs.linaro.org/show_bug.cgi?id=2777#c36

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, 2017-06-23 at 18:32 +0200, Christoffer Dall wrote:
> > You mention in [1] that applying this patch and using a
> > recent QEMU fixes the problem for you, however I can't
> > say the same: I still get
> > 
> >   -device isa-serial,chardev=charserial0,id=serial0:
> >   No 'ISA' bus found for device 'isa-serial'
> 
> Well that's not the bug reported in 2777.  If you try to create an ISA
> bus or configure your domain with an ISA bus on AArch64 your are bound
> to fail, because we never had, and we never will have, support for an
> ISA bus on AArch64.

Sure, I'm aware of that.

> To verify what this patch changes, you can use the test xml file
> listed in [1] as well:
> 
> <domain type='kvm'>
>   <name>testlogfile</name>
>   <memory unit='KiB'>524288</memory>
>   <os>
>     <type arch='aarch64' machine='virt-2.7'>hvm</type>
>   </os>
>   <devices>
>     <serial type='pty'>
>       <log file='/tmp/testlogfile.log' append='off'/>
>       <target port='0'/>
>     </serial>
>   </devices>
> </domain>
> 
> Or any working domain configuration where you add <log file='...' />
> to the domain definition.

In both cases, I get the error reported in my previous
message.

You didn't really answer my question, though: can *you*
start such a guest succesfully using a patched libvirt?
And if so, what is the corresponding QEMU command line?

> It may be that we have an additional bug in libvirt that it under some
> circumstances tries to create an ISA bus with an AArch64 VM, but I
> don't see that being related to the patch above?

It's not.

> Note that the submitted patch fixes virQEMUCapsSupportsChardev, which
> should be independent from any ISA bus fixes in libvirt, but given my
> very limited experience with libvirt, I may be wrong here.

No, you're correct.

> In summary, if your test setup goes from "error: unsupported
> configuration: logfile not supported in this QEMU binary" to "-device
> isa-serial,chardev=charserial0,id=serial0: No 'ISA' bus found for
> device 'isa-serial'" then I'd argue that my patch solves the first
> issue.

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.

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.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Sat, Jun 24, 2017 at 3:37 PM, Andrea Bolognani <abologna@redhat.com> wrote:
> On Fri, 2017-06-23 at 18:32 +0200, Christoffer Dall wrote:
>> > You mention in [1] that applying this patch and using a
>> > recent QEMU fixes the problem for you, however I can't
>> > say the same: I still get
>> >
>> >   -device isa-serial,chardev=charserial0,id=serial0:
>> >   No 'ISA' bus found for device 'isa-serial'
>>
>> Well that's not the bug reported in 2777.  If you try to create an ISA
>> bus or configure your domain with an ISA bus on AArch64 your are bound
>> to fail, because we never had, and we never will have, support for an
>> ISA bus on AArch64.
>
> Sure, I'm aware of that.
>
>> To verify what this patch changes, you can use the test xml file
>> listed in [1] as well:
>>
>> <domain type='kvm'>
>>   <name>testlogfile</name>
>>   <memory unit='KiB'>524288</memory>
>>   <os>
>>     <type arch='aarch64' machine='virt-2.7'>hvm</type>
>>   </os>
>>   <devices>
>>     <serial type='pty'>
>>       <log file='/tmp/testlogfile.log' append='off'/>
>>       <target port='0'/>
>>     </serial>
>>   </devices>
>> </domain>
>>
>> Or any working domain configuration where you add <log file='...' />
>> to the domain definition.
>
> In both cases, I get the error reported in my previous
> message.
>
> You didn't really answer my question, though: can *you*
> start such a guest succesfully using a patched libvirt?

Yes, I can.  But since I now left for holiday and I turned off my
desktop at home, I cannot immediately dig out the details of a config
domain xml file etc.

Riku, did you have a chance to reproduce the issue with my patch as well?

> And if so, what is the corresponding QEMU command line?
>

I'll be happy to dig this out for you when I return though.

>> It may be that we have an additional bug in libvirt that it under some
>> circumstances tries to create an ISA bus with an AArch64 VM, but I
>> don't see that being related to the patch above?
>
> It's not.
>
>> Note that the submitted patch fixes virQEMUCapsSupportsChardev, which
>> should be independent from any ISA bus fixes in libvirt, but given my
>> very limited experience with libvirt, I may be wrong here.
>
> No, you're correct.
>
>> In summary, if your test setup goes from "error: unsupported
>> configuration: logfile not supported in this QEMU binary" to "-device
>> isa-serial,chardev=charserial0,id=serial0: No 'ISA' bus found for
>> device 'isa-serial'" then I'd argue that my patch solves the first
>> issue.
>
> 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.

In any case, I'll reproduce again when I'm back and send you the details.

>
> 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.  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?

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?

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.

Thanks,
-Christoffer

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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.

> 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 :)

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.

> 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 ;)

> 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.

Enjoy your vacation ^^


[1] https://bugzilla.redhat.com/show_bug.cgi?id=1456882#c4
-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 06/24/2017 10:07 PM, Andrea Bolognani 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.
> 
>> 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 :)
> 
> 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.
> 
>> 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 ;)
> 
>> 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 :)

It may fix part of the issue but it likely breaks all existing aarch64 -M virt
libvirt VMs. My VM created by virt-manager has:

    <serial type='pty'>
      <target port='0'/>
    </serial>
    <console type='pty'>
      <target type='serial' port='0'/>
    </console>

And after this patch fails with:

error: Failed to start domain fedora25-aarch64
error: internal error: process exited while connecting to monitor:
2017-06-26T13:55:34.726293Z qemu-system-aarch64: -chardev pty,id=charserial0:
char device redirected to /dev/pts/5 (label charserial0)
2017-06-26T13:55:34.782121Z qemu-system-aarch64: -device
isa-serial,chardev=charserial0,id=serial0: No 'ISA' bus found for device
'isa-serial'

There's a few things going on here:

- Internally libvirt thinks that by default <serial> is isa-serial
- For arm qemu though it's actually a pl011. This is a platform device and as
such there isn't any current way to use -chardev with it AFAIK.
- However -chardev will work for pci-serial device, which is <serial><target
type='serial'/>

So for one, we should change SupportsChardev to also check the devices target
type. The test suite likely needs to be extended to add QEMU_CAPS_CHARDEV for
all arm/aarch64 test cases, to demonstrate the change. And describe the
existing logic by adding a TARGET_TYPE_PL011 which is filled in as the default
for arm/aarch64, and then we can key off that in the code.

After that I think the options are either:

1) Openstack/other tools specifies target type='pci' for PCIe machvirt, to
make <log> work

2) Change the libvirt serial default to target type=pci for PCIe machvirt. Not
sure if they are operationally equivalent for all cases we care about though.
Maybe we could make some kind of adaptive default like 'if PCIe machvirt &&
serial device has <log> specified (or other features) then default target
type=pci', but maybe that's too magic

Would also be nice to have the libvirt output XML always list the serial
target type which would clear up some of this confusion, but might cause
migration XML compat issues for other archs

In the interim though I think this patch should be reverted.

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Mon, Jun 26, 2017 at 10:10:55AM -0400, Cole Robinson wrote:
> On 06/24/2017 10:07 PM, Andrea Bolognani wrote:
> > On Sat, 2017-06-24 at 16:07 +0200, Christoffer Dall wrote:
> >> 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 :)
> 
> It may fix part of the issue but it likely breaks all existing aarch64 -M virt
> libvirt VMs. My VM created by virt-manager has:
> 
>     <serial type='pty'>
>       <target port='0'/>
>     </serial>
>     <console type='pty'>
>       <target type='serial' port='0'/>
>     </console>
> 
> And after this patch fails with:
> 
> error: Failed to start domain fedora25-aarch64
> error: internal error: process exited while connecting to monitor:
> 2017-06-26T13:55:34.726293Z qemu-system-aarch64: -chardev pty,id=charserial0:
> char device redirected to /dev/pts/5 (label charserial0)
> 2017-06-26T13:55:34.782121Z qemu-system-aarch64: -device
> isa-serial,chardev=charserial0,id=serial0: No 'ISA' bus found for device
> 'isa-serial'
> 
> There's a few things going on here:
> 
> - Internally libvirt thinks that by default <serial> is isa-serial
> - For arm qemu though it's actually a pl011. This is a platform device and as
> such there isn't any current way to use -chardev with it AFAIK.

There is,

 -chardev pty,id=chardev0,logfile=/my/log/file \
 -serial chardev:chardev0

> - However -chardev will work for pci-serial device, which is <serial><target
> type='serial'/>

This works, but would require the guest kernel to have console=ttyS0 on
its command line, and adds an unnecessary extra serial device to the
guest, as it already has the PL011.

> 
> So for one, we should change SupportsChardev to also check the devices target
> type. The test suite likely needs to be extended to add QEMU_CAPS_CHARDEV for
> all arm/aarch64 test cases, to demonstrate the change. And describe the
> existing logic by adding a TARGET_TYPE_PL011 which is filled in as the default
> for arm/aarch64, and then we can key off that in the code.
> 
> After that I think the options are either:
> 
> 1) Openstack/other tools specifies target type='pci' for PCIe machvirt, to
> make <log> work
> 
> 2) Change the libvirt serial default to target type=pci for PCIe machvirt. Not
> sure if they are operationally equivalent for all cases we care about though.
> Maybe we could make some kind of adaptive default like 'if PCIe machvirt &&
> serial device has <log> specified (or other features) then default target
> type=pci', but maybe that's too magic

3) magic, but magic that enables the PL011 to be the one and only default
   serial console device for mach-virt. The magic I advocate using is the
   -serial 'chardev:<chardev>' shown above.

> 
> Would also be nice to have the libvirt output XML always list the serial
> target type which would clear up some of this confusion, but might cause
> migration XML compat issues for other archs
> 
> In the interim though I think this patch should be reverted.

Reverted, or quickly completed. It's not wrong, but apparently it needs to
also ensure pci-serial is chosen for ARM guest targets.

Thanks,
drew

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 06/26/2017 11:19 AM, Andrew Jones wrote:
> On Mon, Jun 26, 2017 at 10:10:55AM -0400, Cole Robinson wrote:
>> On 06/24/2017 10:07 PM, Andrea Bolognani wrote:
>>> On Sat, 2017-06-24 at 16:07 +0200, Christoffer Dall wrote:
>>>> 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 :)
>>
>> It may fix part of the issue but it likely breaks all existing aarch64 -M virt
>> libvirt VMs. My VM created by virt-manager has:
>>
>>     <serial type='pty'>
>>       <target port='0'/>
>>     </serial>
>>     <console type='pty'>
>>       <target type='serial' port='0'/>
>>     </console>
>>
>> And after this patch fails with:
>>
>> error: Failed to start domain fedora25-aarch64
>> error: internal error: process exited while connecting to monitor:
>> 2017-06-26T13:55:34.726293Z qemu-system-aarch64: -chardev pty,id=charserial0:
>> char device redirected to /dev/pts/5 (label charserial0)
>> 2017-06-26T13:55:34.782121Z qemu-system-aarch64: -device
>> isa-serial,chardev=charserial0,id=serial0: No 'ISA' bus found for device
>> 'isa-serial'
>>
>> There's a few things going on here:
>>
>> - Internally libvirt thinks that by default <serial> is isa-serial
>> - For arm qemu though it's actually a pl011. This is a platform device and as
>> such there isn't any current way to use -chardev with it AFAIK.
> 
> There is,
> 
>  -chardev pty,id=chardev0,logfile=/my/log/file \
>  -serial chardev:chardev0
> 

Ah interesting I didn't know about that, thanks. I sent some patches that
convert to use that method for platform serial devices

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list