On Tue, 2018-05-15 at 12:53 +0200, Lubomir Rintel wrote: > What works, with images from [1]: > > virt-install \ > --import --name riscv64 \ > --arch riscv64 --machine virt \ > --memory 2048 \ > --rng /dev/urandom \ > --disk /var/lib/libvirt/images/stage4-disk.img,bus=virtio \ > --boot kernel=/var/lib/libvirt/images/bbl,kernel_args="console=ttyS0 ro root=/dev/vda" > > [1] https://fedorapeople.org/groups/risc-v/disk-images/ > > Signed-off-by: Lubomir Rintel <lkundrak@xxxxx> > --- > docs/schemas/basictypes.rng | 2 ++ > src/qemu/qemu_command.c | 15 ++++++++------- > src/qemu/qemu_domain.c | 18 +++++++++++++++--- > src/qemu/qemu_domain_address.c | 21 +++++++++++---------- > src/util/virarch.c | 3 +++ > src/util/virarch.h | 6 ++++++ > 6 files changed, 45 insertions(+), 20 deletions(-) This patch breaks 'make check'. Please make sure that both 'make check' and 'make syntax-check' pass after every single patch in the series - it's a requirement for getting it merged. I'll wait for a respin that addresses that before moving forward with the review, but I'd like to make a few comments on the series as a whole. The way you've organized your changes could use some improvements: for example, in this patch you're both teaching libvirt about the RISC-V architecture and changing guest behavior, which is basically two or more changes squashed into a single patch; moreover, I would expect the former to happen fairly early in the series, certainly before introducing a RISC-V specific serial device. Squashing patches together is easier than disentangling unrelated changes, so when in doubt always err on the side of small patches ;) Overall, I would expect the series to be organized along the lines of * preparatory work, such as creating a wrapper around qemuDomainAssignARMVirtioMMIOAddresses(); * teaching libvirt about the RISC-V architecture; * teaching libvirt about the new serial console model (and potentially target type); * putting it all together; * updating the test suite. I'm not saying you have to follow this to the letter, and more specifically some changes to the test suite might need to happen at the same time as you add RISC-V support, but overall it should end up looking similar to that. Looking forward to v2 :) -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
