Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add device address parameter for s390x #4010

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maxujun
Copy link
Contributor

@maxujun maxujun commented Oct 8, 2024

ID:1360

@maxujun
Copy link
Contributor Author

maxujun commented Oct 8, 2024

(1/1) Host_RHEL.m8.u10.product_rhel.nographic.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.10.0.s390x.io-github-autotest-qemu.boot.s390-virtio: STARTED
(1/1) Host_RHEL.m8.u10.product_rhel.nographic.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.8.10.0.s390x.io-github-autotest-qemu.boot.s390-virtio: PASS (26.29 s)

@maxujun maxujun marked this pull request as draft October 29, 2024 09:08
@maxujun maxujun force-pushed the devno branch 2 times, most recently from e6f0fc6 to bb0897e Compare December 20, 2024 07:52
@maxujun maxujun marked this pull request as ready for review December 25, 2024 12:11
@maxujun
Copy link
Contributor Author

maxujun commented Dec 25, 2024

@PaulYuuu @fbq815 Could you help review?

Copy link
Contributor

@PaulYuuu PaulYuuu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maxujun code almost good to me, but could you avoid handle blank lines on qdevices.py, as it's not a part of this PR.

And I leave few comments for review, also I saw 1 CI failed you need to take a look.

def __init__(self, busid, bus_type, aobject):
"""bus&ssid&devno, 4 ssid and 65536 device numbers"""
super(QCSSBus, self).__init__(
"bus", [["ssid", "devno"], [4, 65536]], busid, bus_type, aobject
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, here only cover ssid and devno, as cssidset fe by default. However, should we cover unusual test scenarios? from the qemu doc

* a virtio-mouse device in a non-standard channel subsystem image::

    -device virtio-mouse-ccw,devno=2.0.2222

  This would not show up in a standard Linux guest.

  The properties for the device would be ``dev_id = "2.0.2222"`` and
  ``subch_id = "2.0.0000"``.

* a virtio-keyboard device in another non-standard channel subsystem image::

    -device virtio-keyboard-ccw,devno=0.0.1234

  This would not show up in a standard Linux guest, either, as ``0`` is not
  the standard channel subsystem image id.

  The properties for the device would be ``dev_id = "0.0.1234"`` and
  ``subch_id = "0.0.0000"``.

we can set cssid value rather than default fe, so how about includes the ssid but set fe by default.

if device.get_param("addr"):
del device.params["addr"]
device.set_param("devno", "fe.%s.%s" % (
hex(addr[0])[2:], f"{addr[1]:04x}"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as we can mark sure the ssid only with length 0-3, so hex(addr[0])[2:] can easy transfer to addr[0]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And for f"{addr[1]:04x}"), of course your can "fe.%s.%4x" and addr[1] to do the same thing. Or just f"fe.{addr[0]}.{addr[1]:4x}"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants