Code review comment for lp:~qzhang/lava-dispatcher/test-abrek

Revision history for this message
Paul Larson (pwlars) wrote :

> > 12 + tester_str = "root@localhost:"
> > As you mentioned previously, we should probably define this elsewhere and
> just import it for consistency. OK for now, but let's do a patch later to
> clean this up everywhere, rather than doing it piecemeal.
> Ok. Here I have a question, if I send a mp to mainline and it is
> accepted, can I go on commit on the branch and send mp again?
Right, just do a new merge proposal for that. It should be a pretty quick one.

> I use an alpha version for master image to test, actually, cron is
> running in master image, and no service run from test image, but after
> chroot, it is still running and test image can see it, so I met the
> error. If cron won't run in our master image finally, I'll remove it
> now.
The reason you see cron running is because you mounted proc. :) That should go away when you remove that extra mount.

> > 87 + self.client.run_shell_command(
> > 88 + 'chroot /mnt/root umount /proc',
> > 89 + response = master_str)
> > There's more cleanup you need to do here. You have a bind mount for /dev,
> and would probably be good to remove /abrek since it was just needed for the
> install, replace resolv.conf original file,
> For /dev bind mount, it doesn't help to eliminate the warning message
> "Can not write log, openpty() failed (/dev/pts not mounted?), does not
> work" after I tried. Do you have some other way? Or we can ignore it for
> all seems working.
The reason is because you used -rbind. You have additional mountpoints, probably for shm and pts, under your remounted /dev. If the error doesn't cause any problem, I think it's safe to ignore. Otherwise, you'll need to either manually mount /dev/pts, and just unmount that when done. Or, you can do something like:
cat /proc/mounts | awk '{print $2}' | grep "^/mnt/root/dev" | sort -r | xargs umount

« Back to merge proposal